Skip to content

Commit 85105c9

Browse files
Fix yarn classic empty-range lock keys (#1271) (#1274)
* Start fix for #1271 Assisted-by: Claude Code:claude-opus-5-5 * Patch yarn 1 lock blocks with an empty range A dependency declared as "left-pad": "" is valid npm semver (the same as "*"), and yarn 1 locks it under the key `left-pad@:`, merged with other members' ranges as `left-pad@, left-pad@^1.3.0:`. The shared yarn key parser rejected a pattern with an empty range, so every classic reader dropped that block: a lock-only scan reported nothing to patch, vendored mode failed with vendor_lock_entry_not_found, and hosted mode pinned nothing while exiting 0. Add a classic-grammar splitter that keeps an empty range and route the classic readers and writers through it (lock inventory, vendored and hosted rewriters, upstream restore, lockfile VEX, copy-source classification). Berry keeps the strict parser: it never writes a rangeless descriptor and skips one as malformed. Fixes #1271 Assisted-by: Claude Code:claude-opus-5-5 * Move empty-range tests out of other test bodies Three of the new #1271 tests and the VEX one landed inside a neighbouring test body or between another test's doc comment and its function, so they compiled as nested items and never ran. Move each to its own top-level spot; all of them run and pass now. Assisted-by: Claude Code:claude-opus-5-5 * Repin live minimist suites to the republished patch Port of #1302 (fixes #1293). Production withdrew the free minimist@1.2.2 patch 80630680-… and republished the same fix as 642d7f02-…, with a new patched index.js (afterHash ec956dca…). That turned hosted-e2e and e2e_safety_pnpm red on main and here. No-ops once main carries #1302 or #1301. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mzbk9WDZkhatFrCWUAyaNs * Accept unprefixed patch file keys in the vlt backtest The republished minimist patch (642d7f02-…) lists its files without npm's `package/` prefix. `holds()` split every key on its first `/`, so a bare `index.js` raised IndexError and every native vlt cell errored. Strip the prefix only when present, like the CLI (patch/apply.rs) and backtest-bun.py's oracle already do. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mzbk9WDZkhatFrCWUAyaNs --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 96312df commit 85105c9

8 files changed

Lines changed: 322 additions & 34 deletions

File tree

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

Lines changed: 115 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -251,6 +251,17 @@ enum HostedDriver {
251251
GetUuid,
252252
}
253253

254+
/// The project the fixture installs.
255+
#[derive(Clone, Copy, PartialEq, Debug)]
256+
enum Layout {
257+
/// One manifest depending on `left-pad@1.3.0`.
258+
Single,
259+
/// #1271: a yarn workspace whose member `a` declares `"left-pad": ""`
260+
/// (an empty range, the same as `*`) and member `b` `"^1.3.0"`, so yarn
261+
/// locks both under one `left-pad@, left-pad@^1.3.0:` block.
262+
EmptyRangeWorkspace,
263+
}
264+
254265
/// Where the fixture configures `yarn-offline-mirror`.
255266
#[derive(Clone, Copy, PartialEq, Debug)]
256267
enum Mirror {
@@ -278,6 +289,7 @@ async fn classic_hosted_project(
278289
tamper_served_tarball: bool,
279290
mirror: Mirror,
280291
driver: HostedDriver,
292+
layout: Layout,
281293
) -> Option<ClassicRedirectFixture> {
282294
let offline_mirror = mirror != Mirror::None;
283295
if !require_yarn_classic(&format!("e2e_redirect_yarn_classic_build ({tag})"), |c| {
@@ -288,13 +300,32 @@ async fn classic_hosted_project(
288300
let tmp = tempfile::tempdir().unwrap();
289301
let proj = tmp.path().join("proj");
290302
std::fs::create_dir_all(&proj).unwrap();
291-
std::fs::write(
292-
proj.join("package.json"),
293-
format!(
294-
r#"{{"name":"redirect-classic-capstone","version":"0.0.0","private":true,"dependencies":{{"{DEP}":"{DEP_VERSION}"}}}}"#
295-
),
296-
)
297-
.unwrap();
303+
match layout {
304+
Layout::Single => std::fs::write(
305+
proj.join("package.json"),
306+
format!(
307+
r#"{{"name":"redirect-classic-capstone","version":"0.0.0","private":true,"dependencies":{{"{DEP}":"{DEP_VERSION}"}}}}"#
308+
),
309+
)
310+
.unwrap(),
311+
Layout::EmptyRangeWorkspace => {
312+
std::fs::write(
313+
proj.join("package.json"),
314+
r#"{"name":"redirect-classic-capstone","version":"0.0.0","private":true,"workspaces":["a","b"]}"#,
315+
)
316+
.unwrap();
317+
for (member, range) in [("a", ""), ("b", "^1.3.0")] {
318+
std::fs::create_dir_all(proj.join(member)).unwrap();
319+
std::fs::write(
320+
proj.join(member).join("package.json"),
321+
format!(
322+
r#"{{"name":"{member}","version":"1.0.0","dependencies":{{"{DEP}":"{range}"}}}}"#
323+
),
324+
)
325+
.unwrap();
326+
}
327+
}
328+
}
298329
let mirror_dir = proj.join("mirror");
299330
let mirror_dir = mirror_dir.to_str().unwrap();
300331
// Where yarn reads the mirror from; the env leg sets it for yarn AND
@@ -356,6 +387,12 @@ async fn classic_hosted_project(
356387
lock_pristine.contains("# yarn lockfile v1"),
357388
"fixture must be a yarn classic v1 lock:\n{lock_pristine}"
358389
);
390+
if layout == Layout::EmptyRangeWorkspace {
391+
assert!(
392+
lock_pristine.contains(&format!("\n{DEP}@, {DEP}@^1.3.0:\n")),
393+
"yarn must merge the empty range into one block:\n{lock_pristine}"
394+
);
395+
}
359396

360397
// 2. Patched tarball + the exact hashes classic will verify at install.
361398
let tgz_path = tmp.path().join(format!("{DEP}-{DEP_VERSION}.tgz"));
@@ -591,6 +628,14 @@ fn fresh_checkout_yarn_install(fx: &ClassicRedirectFixture) -> (PathBuf, Output)
591628
std::fs::create_dir_all(&fresh).unwrap();
592629
std::fs::copy(fx.proj.join("package.json"), fresh.join("package.json")).unwrap();
593630
std::fs::copy(fx.proj.join("yarn.lock"), fresh.join("yarn.lock")).unwrap();
631+
// Workspace members' manifests (Layout::EmptyRangeWorkspace).
632+
for member in ["a", "b"] {
633+
let manifest = fx.proj.join(member).join("package.json");
634+
if manifest.is_file() {
635+
std::fs::create_dir_all(fresh.join(member)).unwrap();
636+
std::fs::copy(manifest, fresh.join(member).join("package.json")).unwrap();
637+
}
638+
}
594639
// v5 hosted mode writes nothing under `.socket/`; carry it when present.
595640
if fx.proj.join(".socket").is_dir() {
596641
copy_dir_recursive(&fx.proj.join(".socket"), &fresh.join(".socket"));
@@ -773,7 +818,14 @@ fn hosted_dev_resave_vex(fx: &ClassicRedirectFixture) {
773818
#[tokio::test(flavor = "multi_thread")]
774819
#[serial_test::serial]
775820
async fn classic_redirect_fresh_checkout_installs_patched_bytes() {
776-
let Some(fx) = classic_hosted_project("main", false, Mirror::None, HostedDriver::Scan).await
821+
let Some(fx) = classic_hosted_project(
822+
"main",
823+
false,
824+
Mirror::None,
825+
HostedDriver::Scan,
826+
Layout::Single,
827+
)
828+
.await
777829
else {
778830
return;
779831
};
@@ -802,6 +854,41 @@ async fn classic_redirect_fresh_checkout_installs_patched_bytes() {
802854
tokio::task::block_in_place(|| hosted_dev_resave_vex(&fx));
803855
}
804856

857+
/// #1271: yarn 1 locks a member's `"left-pad": ""` (an empty range) under
858+
/// one `left-pad@, left-pad@^1.3.0:` block with the other member's range.
859+
/// That block is the installed registry copy: the hosted scan pins it (the
860+
/// fixture asserts one redirect and the lock pin), and a fresh checkout
861+
/// installs the patched bytes for both members.
862+
#[tokio::test(flavor = "multi_thread")]
863+
#[serial_test::serial]
864+
async fn classic_empty_range_workspace_key_is_pinned() {
865+
let Some(fx) = classic_hosted_project(
866+
"empty-range",
867+
false,
868+
Mirror::None,
869+
HostedDriver::Scan,
870+
Layout::EmptyRangeWorkspace,
871+
)
872+
.await
873+
else {
874+
return;
875+
};
876+
let lock = std::fs::read_to_string(fx.proj.join("yarn.lock")).unwrap();
877+
assert!(
878+
lock.contains(&format!("\n{DEP}@, {DEP}@^1.3.0:\n")),
879+
"the pin keeps the key line:\n{lock}"
880+
);
881+
let (fresh, ci) = fresh_checkout_yarn_install(&fx);
882+
assert!(
883+
ci.status.success(),
884+
"fresh-checkout install must succeed.\nstdout:\n{}\nstderr:\n{}",
885+
String::from_utf8_lossy(&ci.stdout),
886+
String::from_utf8_lossy(&ci.stderr),
887+
);
888+
let installed = std::fs::read(fresh.join("node_modules").join(DEP).join("index.js")).unwrap();
889+
assert_eq!(installed, fx.patched, "both members load the patched bytes");
890+
}
891+
805892
/// get-driven hosted twin (v4.0): `get <uuid> --mode hosted --json --yes`
806893
/// routes through the SAME hosted engine as `scan --mode hosted`, so the
807894
/// classic chain must hold unchanged — the fixture's lock pin (hosted URL +
@@ -813,8 +900,14 @@ async fn classic_redirect_fresh_checkout_installs_patched_bytes() {
813900
#[tokio::test(flavor = "multi_thread")]
814901
#[serial_test::serial]
815902
async fn classic_get_uuid_hosted_fresh_checkout_installs() {
816-
let Some(fx) =
817-
classic_hosted_project("get-uuid", false, Mirror::None, HostedDriver::GetUuid).await
903+
let Some(fx) = classic_hosted_project(
904+
"get-uuid",
905+
false,
906+
Mirror::None,
907+
HostedDriver::GetUuid,
908+
Layout::Single,
909+
)
910+
.await
818911
else {
819912
return;
820913
};
@@ -847,7 +940,14 @@ async fn classic_get_uuid_hosted_fresh_checkout_installs() {
847940
#[tokio::test(flavor = "multi_thread")]
848941
#[serial_test::serial]
849942
async fn classic_redirect_tampered_hosted_tarball_fails_integrity() {
850-
let Some(fx) = classic_hosted_project("tampered", true, Mirror::None, HostedDriver::Scan).await
943+
let Some(fx) = classic_hosted_project(
944+
"tampered",
945+
true,
946+
Mirror::None,
947+
HostedDriver::Scan,
948+
Layout::Single,
949+
)
950+
.await
851951
else {
852952
return;
853953
};
@@ -894,6 +994,7 @@ async fn classic_offline_mirror_refuses_hosted_and_keeps_installs_working() {
894994
false,
895995
Mirror::ProjectRc,
896996
HostedDriver::Scan,
997+
Layout::Single,
897998
)
898999
.await
8991000
else {
@@ -975,7 +1076,9 @@ async fn classic_offline_mirror_outside_project_rc_refuses_hosted() {
9751076
("offline-mirror-parent", Mirror::ParentRc),
9761077
("offline-mirror-env", Mirror::Env),
9771078
] {
978-
let Some(fx) = classic_hosted_project(tag, false, mirror, HostedDriver::Scan).await else {
1079+
let Some(fx) =
1080+
classic_hosted_project(tag, false, mirror, HostedDriver::Scan, Layout::Single).await
1081+
else {
9791082
continue;
9801083
};
9811084
assert!(

‎crates/socket-patch-core/src/formats/yarn/patterns.rs‎

Lines changed: 61 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -66,12 +66,38 @@ pub(crate) fn split_pattern(pattern: &str) -> Option<(&str, &str)> {
6666
Some((name, range))
6767
}
6868

69+
/// Split a classic key pattern `name@range` like [`split_pattern`], but
70+
/// keep an EMPTY range (#1271): `""` is valid npm semver (the same as `*`),
71+
/// and yarn 1 locks `"left-pad": ""` under `left-pad@:` — merged with other
72+
/// ranges as `left-pad@, left-pad@^1.3.0:`. Berry never writes a rangeless
73+
/// descriptor (its ranges carry a protocol), so the berry readers keep the
74+
/// strict [`split_pattern`] and treat `name@` as malformed.
75+
pub(crate) fn split_classic_pattern(pattern: &str) -> Option<(&str, &str)> {
76+
let from = usize::from(pattern.starts_with('@'));
77+
let at = pattern[from..].find('@')? + from;
78+
let (name, range) = (&pattern[..at], &pattern[at + 1..]);
79+
if name.is_empty() {
80+
return None;
81+
}
82+
Some((name, range))
83+
}
84+
6985
/// The real package a key pattern stands for: its name, unless the range is
7086
/// an `npm:` alias — then the aliased target's name.
7187
pub(crate) fn pattern_real_name(pattern: &str) -> Option<&str> {
72-
let (name, range) = split_pattern(pattern)?;
88+
real_name_with(pattern, split_pattern)
89+
}
90+
91+
/// [`pattern_real_name`] over the classic grammar ([`split_classic_pattern`]):
92+
/// a rangeless `left-pad@` stands for `left-pad`.
93+
pub(crate) fn classic_pattern_real_name(pattern: &str) -> Option<&str> {
94+
real_name_with(pattern, split_classic_pattern)
95+
}
96+
97+
fn real_name_with(pattern: &str, split: fn(&str) -> Option<(&str, &str)>) -> Option<&str> {
98+
let (name, range) = split(pattern)?;
7399
if let Some(aliased) = range.strip_prefix("npm:") {
74-
return match split_pattern(aliased) {
100+
return match split(aliased) {
75101
Some((real, _)) => Some(real),
76102
None => Some(aliased), // `npm:left-pad` with no range
77103
};
@@ -80,10 +106,10 @@ pub(crate) fn pattern_real_name(pattern: &str) -> Option<&str> {
80106
}
81107

82108
/// The one real package EVERY pattern of a classic key stands for
83-
/// ([`pattern_real_name`]): `None` when there is no pattern, one does not
84-
/// parse, or they name different packages.
109+
/// ([`classic_pattern_real_name`]): `None` when there is no pattern, one
110+
/// does not parse, or they name different packages.
85111
pub(crate) fn classic_key_real_name(patterns: &[String]) -> Option<&str> {
86-
let mut names = patterns.iter().map(|p| pattern_real_name(p));
112+
let mut names = patterns.iter().map(|p| classic_pattern_real_name(p));
87113
let first = names.next()??;
88114
names.all(|n| n == Some(first)).then_some(first)
89115
}
@@ -169,6 +195,36 @@ pub(crate) fn berry_npm_alias_target(range: &str) -> Option<&str> {
169195
mod tests {
170196
use super::*;
171197

198+
/// #1271: the classic grammar keeps an empty range (`left-pad@` from
199+
/// `"left-pad": ""`); the strict berry grammar still rejects it.
200+
#[test]
201+
fn classic_patterns_keep_an_empty_range() {
202+
assert_eq!(split_classic_pattern("left-pad@"), Some(("left-pad", "")));
203+
assert_eq!(
204+
split_classic_pattern("@scope/pkg@"),
205+
Some(("@scope/pkg", ""))
206+
);
207+
assert_eq!(
208+
split_classic_pattern("left-pad@^1.3.0"),
209+
Some(("left-pad", "^1.3.0"))
210+
);
211+
assert_eq!(split_classic_pattern("@scope/pkg"), None);
212+
assert_eq!(split_classic_pattern("left-pad"), None);
213+
assert_eq!(split_classic_pattern("@"), None);
214+
assert_eq!(split_pattern("left-pad@"), None);
215+
216+
assert_eq!(classic_pattern_real_name("left-pad@"), Some("left-pad"));
217+
assert_eq!(
218+
classic_pattern_real_name("lp@npm:left-pad@"),
219+
Some("left-pad")
220+
);
221+
let merged = split_key_patterns("left-pad@, left-pad@^1.3.0");
222+
assert_eq!(merged, ["left-pad@", "left-pad@^1.3.0"]);
223+
assert_eq!(classic_key_real_name(&merged), Some("left-pad"));
224+
let quoted = split_key_patterns("\"@scope/pkg@\", \"@scope/pkg@^1.0.0\"");
225+
assert_eq!(classic_key_real_name(&quoted), Some("@scope/pkg"));
226+
}
227+
172228
#[test]
173229
fn resolution_selector_targets() {
174230
for (sel, want) in [

‎crates/socket-patch-core/src/formats/yarn/source.rs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
//! Where yarn 1 installs a classic lock block's copy from, decided once for
22
//! every mode that reads or rewrites the block.
33
4-
use super::patterns::split_pattern;
4+
use super::patterns::split_classic_pattern;
55
use crate::vendor::npm_origin::npm_spec_is_registry;
66

77
/// Where yarn 1 installs a lock block's copy from: the ONE classifier every
@@ -48,7 +48,7 @@ pub(crate) enum CopySource {
4848
/// [`CopySource`] of a classic block from its key patterns and `resolved`.
4949
pub(crate) fn classic_copy_source(patterns: &[String], resolved: Option<&str>) -> CopySource {
5050
for pattern in patterns {
51-
let range = split_pattern(pattern).map(|(_, r)| r).unwrap_or("");
51+
let range = split_classic_pattern(pattern).map(|(_, r)| r).unwrap_or("");
5252
if range.starts_with("link:") {
5353
return CopySource::Link;
5454
}
@@ -66,7 +66,7 @@ pub(crate) fn classic_copy_source(patterns: &[String], resolved: Option<&str>) -
6666
};
6767
let registry_ranges = patterns
6868
.iter()
69-
.all(|p| split_pattern(p).is_some_and(|(_, range)| npm_spec_is_registry(range)));
69+
.all(|p| split_classic_pattern(p).is_some_and(|(_, range)| npm_spec_is_registry(range)));
7070
if registry_ranges && !is_codeload_tarball(resolved) {
7171
CopySource::Registry
7272
} else {
@@ -81,7 +81,7 @@ pub(crate) fn classic_copy_source(patterns: &[String], resolved: Option<&str>) -
8181
/// a path that leaves the root.
8282
pub(crate) fn classic_file_directory(patterns: &[String]) -> Option<String> {
8383
patterns.iter().find_map(|p| {
84-
let path = split_pattern(p)?.1.strip_prefix("file:")?;
84+
let path = split_classic_pattern(p)?.1.strip_prefix("file:")?;
8585
let path = path.split('#').next().unwrap_or_default();
8686
if is_tarball_path(path) {
8787
return None;
@@ -180,9 +180,9 @@ fn is_codeload_tarball(resolved: &str) -> bool {
180180
/// here: yarn locks them to a codeload tarball and fetches that as one.
181181
pub(crate) fn classic_block_is_git(patterns: &[String], resolved: Option<&str>) -> bool {
182182
patterns.iter().any(|p| {
183-
split_pattern(p).is_some_and(|(_, range)| {
183+
split_classic_pattern(p).is_some_and(|(_, range)| {
184184
let range = match range.strip_prefix("npm:") {
185-
Some(aliased) => split_pattern(aliased).map_or("", |(_, r)| r),
185+
Some(aliased) => split_classic_pattern(aliased).map_or("", |(_, r)| r),
186186
None => range,
187187
};
188188
yarn_classic_range_is_git(range)

0 commit comments

Comments
 (0)