Skip to content

Commit bf1aecc

Browse files
mikolalysenkoclaude
andcommitted
fix(yarn-berry): run the new mode's berry gates before a takeover reverts the old one
The reverts keep line endings as they are (a vendored or hosted revert never refuses on them, so a lock mixed after wiring stays mixed), while the forward hosted rewriter and vendored backend refuse a mixed file. Neither takeover checked first: - `scan`/`get --mode hosted` over a vendored berry purl reverted its wiring, ledger entry and artifact (`redirect_takeover_reverted_vendored`: "now fully hosted"), then the rewriter refused the mixed lock - `redirected: 0`, and the next `yarn install` pulled the unpatched registry package. - `vendor` / `scan --mode vendored` over a hosted berry purl reverted the hosted edits and dropped the redirect-ledger record (`vendor_takeover_reverted_redirect`), then failed `vendor_yarn_berry_mixed_line_endings`. Extract the rewriter's project gates into `redirect::preflight_yarn_berry_hosted` (mixed endings, cacheKey, `.yarnrc.yml` compressionLevel) and the backend's into `vendor::yarn_berry_vendor_preflight` (both files' endings, cacheKey, compressionLevel; berry flavor only), and run each before the matching takeover revert, mirroring the bun preflights - wet and --dry-run alike. A refused purl keeps the old mode's wiring byte-identical and is skipped / failed with the new mode's code. Tests: a hermetic in_process_vendor test drives both directions (mixed lock, mixed package.json, compressionLevel 9; wet and dry-run) and asserts the wiring snapshot is unchanged and no takeover is announced (fails on the pre-fix code in both directions); core unit tests pin that each preflight matches its forward gate's code and detail. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
1 parent 58e34e2 commit bf1aecc

9 files changed

Lines changed: 589 additions & 108 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -595,6 +595,18 @@ into the new version's section — see docs/releasing.md.
595595
4.12.0 (hosted, vendored, workspaces, pnpm linker, both mode takeovers)
596596
with the fixtures re-spelled CRLF, and on yarn 2.4.3 / 3.8.7 (still
597597
refused for their cacheKey, never for their endings).
598+
- **A yarn berry mode takeover no longer strips the old mode's patch before
599+
the new mode refuses the project.** `scan` / `get --mode hosted` over a
600+
vendored berry purl reverted its vendored wiring, ledger entry and
601+
artifact (`redirect_takeover_reverted_vendored`: "now fully hosted") and
602+
only then ran the rewriter, which refused a lock with mixed line endings
603+
(or an unsupported `cacheKey` / `.yarnrc.yml` `compressionLevel`) —
604+
`redirected: 0`, and the next `yarn install` pulled the unpatched registry
605+
package. `vendor` / `scan --mode vendored` over a hosted berry purl did the
606+
same in reverse (`vendor_takeover_reverted_redirect`, then `failed`
607+
`vendor_yarn_berry_mixed_line_endings`). Both takeovers now run the new
608+
mode's berry gates first — wet and `--dry-run` alike — and a refused purl
609+
keeps the old mode's wiring byte-identical.
598610
- **`setup` keeps a CRLF `package.json` CRLF.** `setup` and `setup --remove`
599611
re-serialized `package.json` with bare LF and dropped a leading BOM, so on
600612
a Windows yarn berry project (yarn pretty-prints the manifest with CRLF) a

‎crates/socket-patch-cli/CLI_CONTRACT.md‎

Lines changed: 2 additions & 2 deletions
Large diffs are not rendered by default.

‎crates/socket-patch-cli/src/commands/scan/hosted.rs‎

Lines changed: 64 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1414,10 +1414,62 @@ pub(crate) async fn run_redirect_selected(
14141414
} else {
14151415
None
14161416
};
1417-
// A bun-refused npm purl is never dispatched (see the loop), so its
1418-
// wiring is not a write target here.
1419-
let bun_refused =
1420-
|c: &Candidate| bun_takeover_refusal.is_some() && c.purl.starts_with("pkg:npm/");
1417+
// Yarn berry twin of the bun gate: the berry rewriter's project-level
1418+
// refusals (mixed line endings, cacheKey, `.yarnrc.yml`
1419+
// compressionLevel) must be known before the takeover reverts a
1420+
// vendored berry purl — the vendored revert keeps a mixed lock mixed
1421+
// (it never refuses on line endings), so reverting first stripped
1422+
// the live vendored patch and then the rewriter refused the lock,
1423+
// leaving the package unpatched in both modes. Only entries the
1424+
// vendor ledger wired through the yarn-berry backend are gated (the
1425+
// lock is read only when one exists); an unreadable lock is left to
1426+
// the revert's own diagnostics.
1427+
let berry_entry = |entry: &socket_patch_core::vendor::VendorEntry| {
1428+
entry.ecosystem == "npm" && entry.flavor.as_deref() == Some("yarn-berry")
1429+
};
1430+
let berry_takeover_refusal = if takeover
1431+
.iter()
1432+
.any(|(_, entry)| entry.as_ref().is_some_and(berry_entry))
1433+
{
1434+
match socket_patch_core::utils::fs::read_regular_to_string(
1435+
&common.cwd.join("yarn.lock"),
1436+
)
1437+
.await
1438+
{
1439+
Ok(lock) => {
1440+
let yarnrc = socket_patch_core::utils::fs::read_regular_to_string(
1441+
&common.cwd.join(".yarnrc.yml"),
1442+
)
1443+
.await
1444+
.ok();
1445+
socket_patch_core::patch::redirect::preflight_yarn_berry_hosted(
1446+
&lock,
1447+
yarnrc.as_deref(),
1448+
)
1449+
.err()
1450+
}
1451+
Err(_) => None,
1452+
}
1453+
} else {
1454+
None
1455+
};
1456+
// The takeover refusal (if any) for one candidate: bun gates every
1457+
// npm purl, berry only its vendored-berry entries. A refused purl is
1458+
// never dispatched (see the loop), so its wiring is not a write
1459+
// target here.
1460+
let takeover_refusal =
1461+
|c: &Candidate,
1462+
entry: Option<&socket_patch_core::vendor::VendorEntry>|
1463+
-> Option<&socket_patch_core::patch::redirect::RewriteWarning> {
1464+
if !c.purl.starts_with("pkg:npm/") {
1465+
return None;
1466+
}
1467+
bun_takeover_refusal.as_ref().or_else(|| {
1468+
berry_takeover_refusal
1469+
.as_ref()
1470+
.filter(|_| entry.is_some_and(berry_entry))
1471+
})
1472+
};
14211473
// SYMLINK PRE-CHECK for the takeover reverts — the same rule as the
14221474
// SYMLINK GUARD below, applied to the files the reverts rewrite
14231475
// (each ledger entry's recorded wiring): the revert backends stage
@@ -1428,7 +1480,11 @@ pub(crate) async fn run_redirect_selected(
14281480
// (and under --dry-run too) so "nothing was written" stays true.
14291481
let revert_targets = takeover
14301482
.iter()
1431-
.filter_map(|(c, entry)| entry.as_ref().filter(|_| !bun_refused(c)))
1483+
.filter_map(|(c, entry)| {
1484+
entry
1485+
.as_ref()
1486+
.filter(|e| takeover_refusal(c, Some(e)).is_none())
1487+
})
14321488
.flat_map(|entry| entry.wiring.iter().map(|w| w.file.as_str()));
14331489
if let Some(linked) =
14341490
socket_patch_core::utils::fs::first_symlink(&common.cwd, revert_targets).await
@@ -1442,10 +1498,7 @@ pub(crate) async fn run_redirect_selected(
14421498
let purl = &candidate.purl;
14431499
let uuid = &candidate.dep.patch_uuid;
14441500
if let Some(entry) = ledger_entry {
1445-
if let Some(warning) = bun_takeover_refusal
1446-
.as_ref()
1447-
.filter(|_| bun_refused(candidate))
1448-
{
1501+
if let Some(warning) = takeover_refusal(candidate, Some(entry)) {
14491502
refused.push(purl.clone());
14501503
if !takeover_pre_warnings
14511504
.iter()
@@ -1581,10 +1634,8 @@ pub(crate) async fn run_redirect_selected(
15811634
}
15821635
}
15831636
for purl in &refused {
1584-
if let Some(c) = candidates.iter().find(|c| &c.purl == purl) {
1585-
let reason = bun_takeover_refusal
1586-
.as_ref()
1587-
.filter(|_| bun_refused(c))
1637+
if let Some((c, entry)) = takeover.iter().find(|(c, _)| &c.purl == purl) {
1638+
let reason = takeover_refusal(c, entry.as_ref())
15881639
.map_or("vendored_revert_failed", |w| w.code.as_str());
15891640
skipped.push(serde_json::json!({
15901641
"purl": purl, "uuid": c.dep.patch_uuid, "reason": reason,

‎crates/socket-patch-cli/src/commands/vendor.rs‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1459,6 +1459,16 @@ pub(crate) async fn vendor_records(
14591459
Err(corrupt) => (None, Some(corrupt)),
14601460
};
14611461

1462+
// Yarn berry takeover preflight (see
1463+
// `socket_patch_core::vendor::yarn_berry_vendor_preflight`): the berry
1464+
// backend's project-level refusals (mixed line endings in yarn.lock or
1465+
// package.json, cacheKey, `.yarnrc.yml` compressionLevel), computed at
1466+
// most once per run and only when a hosted-claimed npm purl reaches the
1467+
// takeover below — which must refuse such a purl BEFORE reverting its
1468+
// hosted edits: a hosted revert keeps a mixed lock mixed, so the backend
1469+
// then refused it with the redirect already gone.
1470+
let berry_takeover_refusal: tokio::sync::OnceCell<Option<(&'static str, String)>> =
1471+
tokio::sync::OnceCell::new();
14621472
let pipenv_version = tokio::sync::OnceCell::new();
14631473
let mut dry_in_sync: u32 = 0;
14641474
// Sorted, so per-package lines print in the same order every run.
@@ -1575,6 +1585,26 @@ pub(crate) async fn vendor_records(
15751585
.keys()
15761586
.any(|k| canonical_purl(k) == canonical_purl(candidate))
15771587
});
1588+
// The refusal the berry backend would raise after the
1589+
// revert, raised HERE instead — the same `failed` event,
1590+
// code and detail, in the dry run and the wet run alike —
1591+
// so the hosted wiring and redirect ledger stay untouched.
1592+
if claimed && candidate.starts_with("pkg:npm/") {
1593+
let refusal = berry_takeover_refusal
1594+
.get_or_init(|| {
1595+
socket_patch_core::vendor::yarn_berry_vendor_preflight(&common.cwd)
1596+
})
1597+
.await;
1598+
if let Some((code, detail)) = refusal {
1599+
has_errors = true;
1600+
env.record(
1601+
PatchEvent::new(PatchAction::Failed, candidate.clone())
1602+
.with_error(*code, detail.clone()),
1603+
);
1604+
report_vendor_failure(common, candidate, detail);
1605+
continue;
1606+
}
1607+
}
15781608
if claimed && common.dry_run {
15791609
// Probe the takeover exactly as the wet run would — the
15801610
// per-purl revert's dry run resolves every inverse and

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

Lines changed: 203 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1207,6 +1207,209 @@ async fn berry_crlf_takeovers_round_trip_both_directions() {
12071207
);
12081208
}
12091209

1210+
/// `scan --mode hosted --json --yes <extra...>` through the binary.
1211+
fn hosted_scan_cli_with(root: &Path, api_url: &str, extra: &[&str]) -> (i32, Value) {
1212+
let mut args = vec![
1213+
"scan",
1214+
"--mode",
1215+
"hosted",
1216+
"--json",
1217+
"--yes",
1218+
"--cwd",
1219+
root.to_str().unwrap(),
1220+
"--api-url",
1221+
api_url,
1222+
"--org",
1223+
"test-org",
1224+
"--api-token",
1225+
"fake-token",
1226+
];
1227+
args.extend_from_slice(extra);
1228+
let (code, stdout, stderr) = run_cli(root, &args, &[]);
1229+
let env: Value = serde_json::from_str(stdout.trim()).unwrap_or_else(|e| {
1230+
panic!(
1231+
"scan --mode hosted --json must emit JSON: {e}\nstdout:\n{stdout}\nstderr:\n{stderr}"
1232+
)
1233+
});
1234+
(code, env)
1235+
}
1236+
1237+
/// The mode wiring a takeover would touch: the berry pair, `.yarnrc.yml`,
1238+
/// and everything under `.socket/vendor/` (vendor ledger, artifact, marker,
1239+
/// redirect ledger), as `(relative path, bytes)`.
1240+
fn berry_wiring_snapshot(root: &Path) -> std::collections::BTreeMap<String, Vec<u8>> {
1241+
fn walk(root: &Path, dir: &Path, out: &mut std::collections::BTreeMap<String, Vec<u8>>) {
1242+
let Ok(entries) = std::fs::read_dir(dir) else {
1243+
return;
1244+
};
1245+
for entry in entries.flatten() {
1246+
let path = entry.path();
1247+
if path.is_dir() {
1248+
walk(root, &path, out);
1249+
} else {
1250+
let rel = path
1251+
.strip_prefix(root)
1252+
.unwrap()
1253+
.to_string_lossy()
1254+
.into_owned();
1255+
out.insert(rel, std::fs::read(&path).unwrap());
1256+
}
1257+
}
1258+
}
1259+
let mut out = std::collections::BTreeMap::new();
1260+
for rel in ["package.json", "yarn.lock", ".yarnrc.yml"] {
1261+
out.insert(rel.to_string(), std::fs::read(root.join(rel)).unwrap());
1262+
}
1263+
walk(root, &root.join(".socket/vendor"), &mut out);
1264+
out
1265+
}
1266+
1267+
/// Mode takeovers must refuse a berry project the NEW mode would refuse
1268+
/// BEFORE reverting the OLD mode's wiring. The reverts keep line endings as
1269+
/// they are (a mixed lock stays mixed), while the forward hosted rewriter
1270+
/// and vendored backend refuse a mixed file (and an unsupported
1271+
/// `.yarnrc.yml` compressionLevel) — so reverting first left the package
1272+
/// unpatched in BOTH modes: `scan --mode hosted` reported
1273+
/// `redirect_takeover_reverted_vendored` ("now fully hosted") then
1274+
/// `redirected: 0`; `vendor` reported `vendor_takeover_reverted_redirect`
1275+
/// then failed. Each leg (wet and --dry-run) asserts the old mode's wiring
1276+
/// stays byte-identical, the refusal carries the new mode's code, and no
1277+
/// takeover is announced.
1278+
#[tokio::test]
1279+
async fn berry_takeovers_refuse_before_reverting_the_old_mode() {
1280+
let server = wiremock::MockServer::start().await;
1281+
mount_berry_hosted_api(&server).await;
1282+
let (pkg, lock) = (
1283+
windows_shape(BERRY_WIN_PKG, true),
1284+
windows_shape(&berry_win_lock(), false),
1285+
);
1286+
// Each breakage lands AFTER the old mode is wired. `mix`: an editor
1287+
// saves one header line of `rel` with LF. `compression`: the project
1288+
// opts into a compressionLevel neither mode can reproduce.
1289+
type Break = fn(&Path, &str);
1290+
let mix: Break = |root, rel| {
1291+
let text = std::fs::read_to_string(root.join(rel)).unwrap();
1292+
std::fs::write(root.join(rel), text.replacen("\r\n", "\n", 1)).unwrap();
1293+
};
1294+
let compression: Break = |root, _| {
1295+
std::fs::write(
1296+
root.join(".yarnrc.yml"),
1297+
"nodeLinker: node-modules\r\nenableGlobalCache: false\r\ncompressionLevel: 9\r\n",
1298+
)
1299+
.unwrap();
1300+
};
1301+
1302+
// ── vendored → hosted ──
1303+
for (label, breakage, rel, code) in [
1304+
(
1305+
"mixed lock",
1306+
mix,
1307+
"yarn.lock",
1308+
"redirect_yarn_berry_mixed_line_endings",
1309+
),
1310+
(
1311+
"compressionLevel",
1312+
compression,
1313+
"",
1314+
"redirect_yarn_berry_cache_unsupported",
1315+
),
1316+
] {
1317+
for dry in [true, false] {
1318+
let ctx = format!("vendored→hosted {label} dry={dry}");
1319+
let tmp = tempfile::tempdir().unwrap();
1320+
let root = tmp.path();
1321+
stage_berry_project(root, &pkg, &lock);
1322+
let (exit, env) = vendor_cli(root, &[]);
1323+
assert_eq!(exit, 0, "{ctx}: vendor: {env:#}");
1324+
breakage(root, rel);
1325+
let before = berry_wiring_snapshot(root);
1326+
let extra: &[&str] = if dry { &["--dry-run"] } else { &[] };
1327+
let (_, env) = hosted_scan_cli_with(root, &server.uri(), extra);
1328+
let text = env.to_string();
1329+
assert!(text.contains(code), "{ctx}: refused with {code}: {env:#}");
1330+
for announced in [
1331+
"redirect_takeover_reverted_vendored",
1332+
"redirect_would_revert_vendored",
1333+
] {
1334+
assert!(
1335+
!text.contains(announced),
1336+
"{ctx}: no takeover ({announced}): {env:#}"
1337+
);
1338+
}
1339+
assert_eq!(env["redirect"]["redirected"], 0, "{ctx}: {env:#}");
1340+
let skipped = env["redirect"]["skipped"]
1341+
.as_array()
1342+
.cloned()
1343+
.unwrap_or_default();
1344+
assert!(
1345+
skipped
1346+
.iter()
1347+
.any(|s| s["purl"] == PURL && s["reason"] == code),
1348+
"{ctx}: the purl is skipped with the refusal's code: {env:#}"
1349+
);
1350+
assert_eq!(
1351+
berry_wiring_snapshot(root),
1352+
before,
1353+
"{ctx}: the vendored wiring, ledger and artifact stay byte-identical"
1354+
);
1355+
}
1356+
}
1357+
1358+
// ── hosted → vendored ──
1359+
for (label, breakage, rel, code) in [
1360+
(
1361+
"mixed lock",
1362+
mix,
1363+
"yarn.lock",
1364+
"vendor_yarn_berry_mixed_line_endings",
1365+
),
1366+
(
1367+
"mixed package.json",
1368+
mix,
1369+
"package.json",
1370+
"vendor_yarn_berry_mixed_line_endings",
1371+
),
1372+
(
1373+
"compressionLevel",
1374+
compression,
1375+
"",
1376+
"vendor_yarn_berry_cache_unsupported",
1377+
),
1378+
] {
1379+
for dry in [true, false] {
1380+
let ctx = format!("hosted→vendored {label} dry={dry}");
1381+
let tmp = tempfile::tempdir().unwrap();
1382+
let root = tmp.path();
1383+
stage_berry_project(root, &pkg, &lock);
1384+
let (exit, env) = hosted_scan_cli_with(root, &server.uri(), &[]);
1385+
assert_eq!(exit, 0, "{ctx}: hosted scan: {env:#}");
1386+
assert_eq!(env["redirect"]["redirected"], 1, "{ctx}: {env:#}");
1387+
breakage(root, rel);
1388+
let before = berry_wiring_snapshot(root);
1389+
let extra: &[&str] = if dry { &["--dry-run"] } else { &[] };
1390+
let (exit, env) = vendor_cli(root, extra);
1391+
assert_eq!(exit, 1, "{ctx}: the refusal fails the run: {env:#}");
1392+
let failed = find_event(&env, "failed", Some(code));
1393+
assert_eq!(failed["purl"], PURL, "{ctx}: {failed}");
1394+
let text = env.to_string();
1395+
for announced in [
1396+
"vendor_takeover_reverted_redirect",
1397+
"vendor_would_revert_redirect",
1398+
] {
1399+
assert!(
1400+
!text.contains(announced),
1401+
"{ctx}: no takeover ({announced}): {env:#}"
1402+
);
1403+
}
1404+
assert_eq!(
1405+
berry_wiring_snapshot(root),
1406+
before,
1407+
"{ctx}: the hosted lock edits and redirect ledger stay byte-identical"
1408+
);
1409+
}
1410+
}
1411+
}
1412+
12101413
// ─────────────────────────────────────────────────────────────────────
12111414
// 9. offline with no local source
12121415
// ─────────────────────────────────────────────────────────────────────

0 commit comments

Comments
 (0)