From e39e94617222cb475c027ccff9dbc3abf9425b82 Mon Sep 17 00:00:00 2001 From: socket-patch-agent Date: Mon, 2 Nov 2026 12:42:17 +0000 Subject: [PATCH 1/5] Start fix for #1899 Assisted-by: Claude Code:claude-opus-5-5 From 844a112bb7951d1c82adde29c715bf9245d9e10b Mon Sep 17 00:00:00 2001 From: socket-patch-agent Date: Mon, 2 Nov 2026 12:48:21 +0000 Subject: [PATCH 2/5] Classify npm store children through one helper apply could patch a package copy that scan never reported. Inside a store entry's own node_modules (for example .pnpm//node_modules), the scan never treats a .pnpm, .bun, .deno, .vlt, .store, .modules.yaml or .registry.* child as a store, but the apply/vex resolver did and walked it. Both walks now classify a node_modules child through one StoreKind::classify(name, file_type, in_store_entry), and the resolver passes the store-entry flag the scan already used. An empty .modules.yaml store no longer queues an empty entry list. A regression test covers every store name nested in a store entry. Refs #1899 Assisted-by: Claude Code:claude-opus-5-5 --- .../src/crawlers/npm_crawler.rs | 331 +++++++++++------- 1 file changed, 196 insertions(+), 135 deletions(-) diff --git a/crates/socket-patch-core/src/crawlers/npm_crawler.rs b/crates/socket-patch-core/src/crawlers/npm_crawler.rs index 2d6acefd5..c00f64fa7 100644 --- a/crates/socket-patch-core/src/crawlers/npm_crawler.rs +++ b/crates/socket-patch-core/src/crawlers/npm_crawler.rs @@ -616,6 +616,64 @@ fn pnpm_shaped_store_layout(name: &str) -> Option { .map(|(_, layout)| *layout) } +/// Which package-manager store a `node_modules` child is. The resolver +/// (`find_by_purls`) and the scan (`crawl_all`) both classify a child +/// through [`StoreKind::classify`], so they agree on which children are +/// stores and on the store-entry rule below. +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +enum StoreKind { + /// pnpm's `.pnpm`, Bun's `.bun` or Deno's `.deno` (see + /// [`PNPM_SHAPED_STORES`]). + PnpmShaped(StoreLayout), + /// vlt's `.vlt` store. + Vlt, + /// npm's `install-strategy=linked` `.store`. + NpmLinked, + /// pnpm's `.modules.yaml`: it names a store relocated by + /// `virtualStoreDir`, or else the global virtual store's entries are + /// reached through it. + PnpmModulesYaml, + /// A pnpm <=3 `.` store (see + /// [`is_legacy_pnpm_store_dir_name`]). + LegacyPnpm, +} + +impl StoreKind { + /// The store a `node_modules` child named `name` of type `file_type` + /// is, or `None` for anything else. + /// + /// Inside a store entry's own `node_modules` (`in_store_entry`) no + /// child is a store: a store entry holds packages and links to sibling + /// entries, never a nested store, so a hidden child there is skipped + /// like any other hidden entry. A store name with the wrong file type + /// (a `.pnpm` file, a `.modules.yaml` dir) is not a store either; every + /// store name is hidden, so callers skip it too. + fn classify(name: &str, file_type: Option, in_store_entry: bool) -> Option { + if in_store_entry { + return None; + } + let kind = if let Some(layout) = pnpm_shaped_store_layout(name) { + StoreKind::PnpmShaped(layout) + } else if name == VLT_STORE_NAME { + StoreKind::Vlt + } else if name == NPM_LINKED_STORE_NAME { + StoreKind::NpmLinked + } else if name == PNPM_MODULES_YAML { + StoreKind::PnpmModulesYaml + } else if is_legacy_pnpm_store_dir_name(name) { + StoreKind::LegacyPnpm + } else { + return None; + }; + let file_type = file_type?; + let shaped = match kind { + StoreKind::PnpmModulesYaml => file_type.is_file(), + _ => file_type.is_dir(), + }; + shaped.then_some(kind) + } +} + /// Decode a `.bun` store entry name (`@`, a peer set /// appended as `+`, scoped `@scope+leaf@…`). The pnpm decoder reads /// the name exactly, but would keep a `+` tail as part of the @@ -3029,7 +3087,7 @@ impl NpmCrawler { matched.extend(aliases); } let gvs_member = !store_entry && may_be_gvs_workspace_member(&listing); - let mut nested = Self::collect_nested_node_modules(&nm_path, listing); + let mut nested = Self::collect_nested_node_modules(&nm_path, listing, store_entry); if store_entry { nested.extend(own_package_nested_node_modules_sync(&nm_path)); } @@ -3138,90 +3196,32 @@ impl NpmCrawler { /// /// Entries are examined in parallel; their contributions keep listing /// order. - fn collect_nested_node_modules(nm_path: &Path, listing: Listing) -> Vec { + fn collect_nested_node_modules( + nm_path: &Path, + listing: Listing, + store_entry: bool, + ) -> Vec { let found: Vec> = par_map(listing.entries, |entry| { - Self::nested_node_modules_of(nm_path, entry) + Self::nested_node_modules_of(nm_path, entry, store_entry) }); found.into_iter().flatten().collect() } /// What one listing entry of `nm_path` contributes to the resolver's - /// next level (see [`Self::collect_nested_node_modules`]). - fn nested_node_modules_of(nm_path: &Path, entry: ListedEntry) -> Vec { + /// next level (see [`Self::collect_nested_node_modules`]). `store_entry` + /// says `nm_path` is a store entry's own `node_modules`, whose children + /// are never stores (see [`StoreKind::classify`]), as in the scan. + fn nested_node_modules_of( + nm_path: &Path, + entry: ListedEntry, + store_entry: bool, + ) -> Vec { let name_str = entry.name_str.as_str(); - // pnpm's virtual store. Under the isolated linker the store is - // the ONLY physical home of transitive dependencies: the - // importer's node_modules holds symlinks for direct deps only, - // so a transitive-only target (installed at - // `.pnpm//node_modules/`, runtime-loaded) is - // unreachable through the symlink-free walk above — invisible - // to apply despite being importable. Probe REAL store entries' - // `node_modules`; the name+version match in `find_by_purls` - // keeps aliases and multi-version store entries distinct, and - // BFS order guarantees a root-linked install has already been - // probed (and removed from `pending`) before these are - // dequeued, so a package is never resolved twice. - // Bun's `.bun` and Deno's `.deno` stores share the shape and the - // property (see `PNPM_SHAPED_STORES`). - if let Some(layout) = pnpm_shaped_store_layout(name_str) { - if !entry.file_type.is_some_and(|ft| ft.is_dir()) { - return Vec::new(); - } - let store = nm_path.join(&entry.name); - let entries = StoreEntry::from_dirs(Self::list_pnpm_shaped_store_entries_sync( - &store, layout, false, false, - )); - return vec![NestedNodeModules::StoreEntries(entries)]; - } - // vlt's store has the same transitive-only-home property: every - // package lives at `.vlt//node_modules/` and the - // importer holds only links into it. - if name_str == VLT_STORE_NAME { - if !entry.file_type.is_some_and(|ft| ft.is_dir()) { - return Vec::new(); - } - let entries = Self::list_vlt_store_entries_sync(&nm_path.join(&entry.name)); - return vec![NestedNodeModules::StoreEntries(StoreEntry::vlt(entries))]; - } - // npm's `install-strategy=linked` store: the same transitive-only - // home, at `.store/@-/node_modules/`. - if name_str == NPM_LINKED_STORE_NAME { - if !entry.file_type.is_some_and(|ft| ft.is_dir()) { - return Vec::new(); - } - let entries = StoreEntry::from_dirs(Self::list_npm_store_entries_sync( - &nm_path.join(&entry.name), - false, - )); - return vec![NestedNodeModules::StoreEntries(entries)]; - } - // A pnpm virtual store relocated by `virtualStoreDir`, found - // through the `.modules.yaml` pnpm writes next to it. It may sit - // outside this `node_modules` or under a hidden name the skip - // below would swallow. - if name_str == PNPM_MODULES_YAML { - if !entry.file_type.is_some_and(|ft| ft.is_file()) { - return Vec::new(); - } - let Some(store) = relocated_pnpm_virtual_store_sync(nm_path) else { - let entries = reachable_pnpm_global_virtual_store_entries_sync(nm_path); - return vec![NestedNodeModules::StoreEntries(entries)]; - }; - let entries = StoreEntry::from_dirs(Self::list_pnpm_store_entries_sync(&store, false)); - return vec![NestedNodeModules::StoreEntries(entries)]; - } - // pnpm <=3: the virtual store is a hidden `.` dir - // (there is no `.pnpm` at all) with the same - // transitive-only-deps property, so it gets the same probing. - // Must run before the generic hidden-entry skip below, which - // would otherwise swallow it — leaving every transitive-only - // install unpatchable on those layouts. - if is_legacy_pnpm_store_dir_name(name_str) { - if !entry.file_type.is_some_and(|ft| ft.is_dir()) { - return Vec::new(); - } - let entries = Self::collect_nested_store_entries_sync(&nm_path.join(&entry.name)); - return vec![NestedNodeModules::StoreEntries(StoreEntry::pnpm(entries))]; + if let Some(kind) = StoreKind::classify(name_str, entry.file_type, store_entry) { + return Self::resolver_store_entries(nm_path, &entry.name, kind) + .map(NestedNodeModules::StoreEntries) + .into_iter() + .collect(); } if name_str.starts_with('.') || name_str == "node_modules" { return Vec::new(); @@ -3253,6 +3253,51 @@ impl NpmCrawler { } } + /// The entries of the store `kind` that `nm_path`'s child `name` is, + /// for the resolver to probe; `None` when there are none. + /// + /// Under the isolated linkers a store is the ONLY physical home of + /// transitive dependencies: the importer's `node_modules` holds links + /// for direct deps only, so a transitive-only target (installed at + /// `.pnpm//node_modules/`, runtime-loaded) is unreachable + /// through the link-free walk, and its REAL store entries are probed + /// instead. The name+version match in `find_by_purls` keeps aliases and + /// multi-version store entries distinct, and BFS order guarantees a + /// root-linked install has already been probed (and removed from + /// `pending`) before these are dequeued, so a package is never + /// resolved twice. pnpm's, Bun's and Deno's stores, vlt's + /// `.vlt//node_modules/`, npm's linked + /// `.store/@-/node_modules/`, a pnpm store + /// relocated by `virtualStoreDir` (found through `.modules.yaml`, it may + /// sit outside this `node_modules`) and pnpm <=3's hidden + /// `.` store all have this property. + fn resolver_store_entries( + nm_path: &Path, + name: &OsStr, + kind: StoreKind, + ) -> Option> { + let store = nm_path.join(name); + let entries = match kind { + StoreKind::PnpmShaped(layout) => StoreEntry::from_dirs( + Self::list_pnpm_shaped_store_entries_sync(&store, layout, false, false), + ), + StoreKind::Vlt => StoreEntry::vlt(Self::list_vlt_store_entries_sync(&store)), + StoreKind::NpmLinked => { + StoreEntry::from_dirs(Self::list_npm_store_entries_sync(&store, false)) + } + StoreKind::PnpmModulesYaml => match relocated_pnpm_virtual_store_sync(nm_path) { + Some(store) => { + StoreEntry::from_dirs(Self::list_pnpm_store_entries_sync(&store, false)) + } + None => reachable_pnpm_global_virtual_store_entries_sync(nm_path), + }, + StoreKind::LegacyPnpm => { + StoreEntry::pnpm(Self::collect_nested_store_entries_sync(&store)) + } + }; + (!entries.is_empty()).then_some(entries) + } + /// Whether a virtual-store entry can still hold a pending target. /// A manifest routinely lists packages that simply aren't installed /// here, and probing every entry of a large monorepo store for them @@ -3561,64 +3606,30 @@ impl NpmCrawler { for entry in listing.entries { let name_str = entry.name_str; - // pnpm's virtual store: under the isolated linker it is the - // ONLY physical home of transitive dependencies (the - // importer's node_modules symlinks direct deps only), so - // skipping it as just-another-hidden-dir leaves every - // transitive-only install invisible to scan. Deferred until - // after this loop so root-level entries are inventoried - // first and win the `seen` name@version dedup at their - // importer-root paths. (A store entry's own children never - // include a nested `.pnpm`; under the store-entry policy the - // name falls through to the hidden-entry skip below.) - // Bun's `.bun` and Deno's `.deno` share both the shape and - // the property (see `PNPM_SHAPED_STORES`). - if let Some(layout) = pnpm_shaped_store_layout(&name_str).filter(|_| !store_entry) { - if entry.file_type.is_some_and(|ft| ft.is_dir()) { - pnpm_shaped_stores.push((node_modules_path.join(&name_str), layout)); - } - continue; - } - - // vlt's store, deferred for the same reason: importer links - // win the `seen` dedup at their importer-root paths. - if !store_entry && name_str == VLT_STORE_NAME { - if entry.file_type.is_some_and(|ft| ft.is_dir()) { - vlt_store = Some(node_modules_path.join(&name_str)); - } - continue; - } - - // npm's linked-strategy store, deferred for the same reason. - if !store_entry && name_str == NPM_LINKED_STORE_NAME { - if entry.file_type.is_some_and(|ft| ft.is_dir()) { - npm_store = Some(node_modules_path.join(&name_str)); - } - continue; - } - - // A pnpm virtual store relocated by `virtualStoreDir` (see - // `relocated_pnpm_virtual_store_sync`), deferred like `.pnpm`. - if !store_entry && name_str == PNPM_MODULES_YAML { - if entry.file_type.is_some_and(|ft| ft.is_file()) { - relocated_pnpm_store = relocated_pnpm_virtual_store_sync(node_modules_path); - if relocated_pnpm_store.is_none() { - global_store_entries = - reachable_pnpm_global_virtual_store_entries_sync(node_modules_path); + // A store: under the isolated linkers it is the ONLY physical + // home of transitive dependencies (the importer's node_modules + // links direct deps only), so skipping it as + // just-another-hidden-dir would leave every transitive-only + // install invisible to scan. Every store is deferred until + // after this loop so root-level entries are inventoried first + // and win the `seen` name@version dedup at their importer-root + // paths. A store entry's own children are never stores (see + // `StoreKind::classify`), so under the store-entry policy the + // names fall through to the hidden-entry skip below. + if let Some(kind) = StoreKind::classify(&name_str, entry.file_type, store_entry) { + let store = node_modules_path.join(&name_str); + match kind { + StoreKind::PnpmShaped(layout) => pnpm_shaped_stores.push((store, layout)), + StoreKind::Vlt => vlt_store = Some(store), + StoreKind::NpmLinked => npm_store = Some(store), + StoreKind::PnpmModulesYaml => { + relocated_pnpm_store = relocated_pnpm_virtual_store_sync(node_modules_path); + if relocated_pnpm_store.is_none() { + global_store_entries = + reachable_pnpm_global_virtual_store_entries_sync(node_modules_path); + } } - } - continue; - } - - // pnpm <=3 virtual store (a hidden `.` dir; - // no `.pnpm` exists on those layouts): same - // transitive-only-home property, same deferred scan so - // root-level entries win the `seen` dedup. Must run before - // the hidden-entry skip below, which would otherwise leave - // every transitive-only install invisible to scan. - if !store_entry && is_legacy_pnpm_store_dir_name(&name_str) { - if entry.file_type.is_some_and(|ft| ft.is_dir()) { - legacy_stores.push(node_modules_path.join(&name_str)); + StoreKind::LegacyPnpm => legacy_stores.push(store), } continue; } @@ -6672,6 +6683,56 @@ mod tests { /// `node_modules/.modules.yaml` records where (relative to /// `node_modules`: JSON on pnpm 10+, YAML before). A relocated store /// inside the project is walked like `.pnpm`, wherever it sits. + /// #1899: a store entry's own `node_modules` holds packages and links + /// to sibling entries, never a nested store. Scan always skipped a + /// store-named child there as a hidden entry, but the resolver walked + /// it as a store, so `apply` could patch a copy `scan` never reported. + /// Both now classify through `StoreKind::classify` and agree: such a + /// copy is neither reported nor resolved, for every store name. + #[tokio::test] + async fn test_store_named_child_of_a_store_entry_is_not_a_store() { + for (store_name, nested_entry) in [ + (".pnpm", "hidden@1.0.0/node_modules/hidden"), + (".bun", "hidden@1.0.0/node_modules/hidden"), + (".deno", "hidden@1.0.0/node_modules/hidden"), + (".store", "hidden@1.0.0-abcdef/node_modules/hidden"), + (".registry.npmjs.org", "hidden/1.0.0/node_modules/hidden"), + ] { + let tmp = tempfile::tempdir().unwrap(); + let root: PathBuf = tmp.path().components().collect(); + let nm = root.join("node_modules"); + let entry_nm = nm.join(".pnpm/outer@1.0.0/node_modules"); + let outer = entry_nm.join("outer"); + write_pkg(&outer, "outer", "1.0.0"); + link_dir(&outer, &nm.join("outer")); + let hidden = entry_nm.join(store_name).join(nested_entry); + write_pkg(&hidden, "hidden", "1.0.0"); + + let scanned = scan_paths(&root).await; + assert!( + scanned + .iter() + .any(|(purl, _)| purl == "pkg:npm/outer@1.0.0"), + "{store_name}: {scanned:?}" + ); + assert!( + !scanned + .iter() + .any(|(purl, _)| purl == "pkg:npm/hidden@1.0.0"), + "{store_name}: scan reported a nested store's copy: {scanned:?}" + ); + + let found = NpmCrawler::new() + .find_by_purls(&nm, &["pkg:npm/hidden@1.0.0".to_string()]) + .await + .unwrap(); + assert!( + found.is_empty(), + "{store_name}: resolver walked a nested store: {found:?}" + ); + } + } + #[tokio::test] async fn test_pnpm_relocated_virtual_store_dir_is_walked() { for (modules_yaml, store_rel) in [ From ef6b60b46e1bcbe749400b37e775689e07eaae77 Mon Sep 17 00:00:00 2001 From: socket-patch-agent Date: Mon, 2 Nov 2026 13:16:15 +0000 Subject: [PATCH 3/5] Skip nested stores in apply's peer fan-out too apply's peer-variant fan-out walked up from a patched copy and probed every ancestor node_modules for a store, including a store entry's own node_modules. A peer variant in a nested .pnpm there was patched although scan never reports it. That walk now skips a store entry's node_modules, matching the scan and the resolver. The test oracle for find_by_purls follows the same store-entry rule, and the regression test now also covers .vlt, a .modules.yaml relocated store and the peer fan-out. Review cleanups: the resolver's store listing returns a plain Vec, a misplaced #362 doc comment is back on its own test, and the walk docs point at StoreKind::classify. Refs #1899 Assisted-by: Claude Code:claude-opus-5-5 --- .../src/crawlers/npm_crawler.rs | 90 +++++++++++++------ .../src/crawlers/npm_crawler/oracle.rs | 12 +-- 2 files changed, 70 insertions(+), 32 deletions(-) diff --git a/crates/socket-patch-core/src/crawlers/npm_crawler.rs b/crates/socket-patch-core/src/crawlers/npm_crawler.rs index c00f64fa7..807ed1302 100644 --- a/crates/socket-patch-core/src/crawlers/npm_crawler.rs +++ b/crates/socket-patch-core/src/crawlers/npm_crawler.rs @@ -616,6 +616,24 @@ fn pnpm_shaped_store_layout(name: &str) -> Option { .map(|(_, layout)| *layout) } +/// Whether `dir` is a named store's entry's own `node_modules` +/// (`//node_modules`, `` one of the pnpm-shaped +/// stores, `.vlt` or `.store`), whose children are never stores (see +/// [`StoreKind::classify`]). +fn is_store_entry_node_modules(dir: &Path) -> bool { + dir.file_name() == Some(OsStr::new("node_modules")) + && dir + .parent() + .and_then(Path::parent) + .and_then(Path::file_name) + .and_then(OsStr::to_str) + .is_some_and(|store| { + pnpm_shaped_store_layout(store).is_some() + || store == VLT_STORE_NAME + || store == NPM_LINKED_STORE_NAME + }) +} + /// Which package-manager store a `node_modules` child is. The resolver /// (`find_by_purls`) and the scan (`crawl_all`) both classify a child /// through [`StoreKind::classify`], so they agree on which children are @@ -2929,8 +2947,8 @@ impl NpmCrawler { if pending.is_empty() { return pending; } - // Each dir is tagged `true` when it is a pnpm or vlt store entry's - // `node_modules`. + // Each dir is tagged `true` when it is a store entry's + // `node_modules` (see `StoreKind::classify`). let mut level: Vec<(PathBuf, bool)> = vec![(node_modules_path.to_path_buf(), false)]; let mut visited = HashSet::new(); // The root's relocated Yarn pnpm-linker store (#859), contributed @@ -3218,10 +3236,8 @@ impl NpmCrawler { ) -> Vec { let name_str = entry.name_str.as_str(); if let Some(kind) = StoreKind::classify(name_str, entry.file_type, store_entry) { - return Self::resolver_store_entries(nm_path, &entry.name, kind) - .map(NestedNodeModules::StoreEntries) - .into_iter() - .collect(); + let entries = Self::resolver_store_entries(nm_path, &entry.name, kind); + return vec![NestedNodeModules::StoreEntries(entries)]; } if name_str.starts_with('.') || name_str == "node_modules" { return Vec::new(); @@ -3254,7 +3270,7 @@ impl NpmCrawler { } /// The entries of the store `kind` that `nm_path`'s child `name` is, - /// for the resolver to probe; `None` when there are none. + /// for the resolver to probe. /// /// Under the isolated linkers a store is the ONLY physical home of /// transitive dependencies: the importer's `node_modules` holds links @@ -3271,13 +3287,9 @@ impl NpmCrawler { /// relocated by `virtualStoreDir` (found through `.modules.yaml`, it may /// sit outside this `node_modules`) and pnpm <=3's hidden /// `.` store all have this property. - fn resolver_store_entries( - nm_path: &Path, - name: &OsStr, - kind: StoreKind, - ) -> Option> { + fn resolver_store_entries(nm_path: &Path, name: &OsStr, kind: StoreKind) -> Vec { let store = nm_path.join(name); - let entries = match kind { + match kind { StoreKind::PnpmShaped(layout) => StoreEntry::from_dirs( Self::list_pnpm_shaped_store_entries_sync(&store, layout, false, false), ), @@ -3294,8 +3306,7 @@ impl NpmCrawler { StoreKind::LegacyPnpm => { StoreEntry::pnpm(Self::collect_nested_store_entries_sync(&store)) } - }; - (!entries.is_empty()).then_some(entries) + } } /// Whether a virtual-store entry can still hold a pending target. @@ -3567,14 +3578,14 @@ impl NpmCrawler { /// Gather one `node_modules` directory's scan into [`ScanEvent`]s, in /// the exact order the sequential walk visits it: the directory's own /// entries (each package followed by its nested `node_modules`), then - /// the deferred pnpm and vlt store(s). `listing` is the directory's - /// listing when the caller already read it. `store_entry` selects the - /// one policy bit distinguishing an importer/package tree from a pnpm - /// or vlt store entry's `node_modules`: + /// the deferred store(s). `listing` is the directory's listing when the + /// caller already read it. `store_entry` selects the one policy bit + /// distinguishing an importer/package tree from a store entry's + /// `node_modules`: /// - importer trees accept both directories and symlinks (pnpm and vlt /// link direct deps; `npm link` targets) but never traverse a symlink, - /// and a `.pnpm` (or pnpm <=3 `.`) or `.vlt` child is - /// the virtual store, walked after the loop; + /// and a store child (see [`StoreKind::classify`]) is walked after + /// the loop; /// - a store entry accepts REAL directories only — a symlinked (or, on /// Windows, junctioned) entry there is the package's dependency /// pointing at a sibling store entry, inventoried via that entry, and @@ -4512,7 +4523,9 @@ async fn find_store_peer_variant_copies_reusing( stores.push((StoreLayout::NpmLinked, dir.to_path_buf())); } } - Some("node_modules") => { + // A store entry's own `node_modules` never holds a store + // (see `StoreKind::classify`), so only other ones are probed. + Some("node_modules") if !is_store_entry_node_modules(dir) => { let others = [ (VLT_STORE_NAME, StoreLayout::Vlt), (NPM_LINKED_STORE_NAME, StoreLayout::NpmLinked), @@ -6679,16 +6692,13 @@ mod tests { assert_eq!(found[&scoped_purl][0].namespace.as_deref(), Some("@s")); } - /// #362: pnpm's `virtualStoreDir` moves the virtual store, and - /// `node_modules/.modules.yaml` records where (relative to - /// `node_modules`: JSON on pnpm 10+, YAML before). A relocated store - /// inside the project is walked like `.pnpm`, wherever it sits. /// #1899: a store entry's own `node_modules` holds packages and links /// to sibling entries, never a nested store. Scan always skipped a /// store-named child there as a hidden entry, but the resolver walked /// it as a store, so `apply` could patch a copy `scan` never reported. /// Both now classify through `StoreKind::classify` and agree: such a - /// copy is neither reported nor resolved, for every store name. + /// copy is neither reported, resolved nor reached by the apply peer + /// fan-out, for every store name. #[tokio::test] async fn test_store_named_child_of_a_store_entry_is_not_a_store() { for (store_name, nested_entry) in [ @@ -6697,6 +6707,9 @@ mod tests { (".deno", "hidden@1.0.0/node_modules/hidden"), (".store", "hidden@1.0.0-abcdef/node_modules/hidden"), (".registry.npmjs.org", "hidden/1.0.0/node_modules/hidden"), + (".vlt", "··hidden@1.0.0/node_modules/hidden"), + // `.modules.yaml` names a relocated store, `.vs` here. + (".vs", "hidden@1.0.0/node_modules/hidden"), ] { let tmp = tempfile::tempdir().unwrap(); let root: PathBuf = tmp.path().components().collect(); @@ -6707,6 +6720,19 @@ mod tests { link_dir(&outer, &nm.join("outer")); let hidden = entry_nm.join(store_name).join(nested_entry); write_pkg(&hidden, "hidden", "1.0.0"); + if store_name == ".vs" { + std::fs::write( + entry_nm.join(".modules.yaml"), + "layoutVersion: 5\nvirtualStoreDir: '.vs'\n", + ) + .unwrap(); + } + // A peer variant of `outer` in the nested store: the apply + // fan-out must not reach it either. + let nested_twin = entry_nm.join(".pnpm/outer@1.0.0(react@18.2.0)/node_modules/outer"); + if store_name == ".pnpm" { + write_pkg(&nested_twin, "outer", "1.0.0"); + } let scanned = scan_paths(&root).await; assert!( @@ -6730,9 +6756,19 @@ mod tests { found.is_empty(), "{store_name}: resolver walked a nested store: {found:?}" ); + assert!( + !find_store_peer_variant_copies(&outer) + .await + .contains(&nested_twin), + "{store_name}: peer fan-out walked a nested store" + ); } } + /// #362: pnpm's `virtualStoreDir` moves the virtual store, and + /// `node_modules/.modules.yaml` records where (relative to + /// `node_modules`: JSON on pnpm 10+, YAML before). A relocated store + /// inside the project is walked like `.pnpm`, wherever it sits. #[tokio::test] async fn test_pnpm_relocated_virtual_store_dir_is_walked() { for (modules_yaml, store_rel) in [ diff --git a/crates/socket-patch-core/src/crawlers/npm_crawler/oracle.rs b/crates/socket-patch-core/src/crawlers/npm_crawler/oracle.rs index 5663ba351..e292e6133 100644 --- a/crates/socket-patch-core/src/crawlers/npm_crawler/oracle.rs +++ b/crates/socket-patch-core/src/crawlers/npm_crawler/oracle.rs @@ -267,7 +267,7 @@ impl LegacyNpmCrawler { .map(|t| t.dir_key.as_str()) .collect(); let filter = filter_store_entries.then_some(&unmatched_names); - Self::collect_nested_node_modules(&nm_path, filter, &mut queue).await; + Self::collect_nested_node_modules(&nm_path, store_entry, filter, &mut queue).await; } // Only the targets with zero copies remain "pending" for pass 2. pending.retain(|t| !result.contains_key(&t.purl)); @@ -337,9 +337,11 @@ impl LegacyNpmCrawler { /// store (see below); `pending_names` — `Some(the still-unresolved /// targets' full package names)` — bounds which store entries get /// enqueued, while `None` (the pass-2 fallback of `find_by_purls`) - /// enqueues every store entry. + /// enqueues every store entry. Inside a store entry's own + /// `node_modules` (`store_entry`) no child is a store (#1899). async fn collect_nested_node_modules( nm_path: &Path, + store_entry: bool, pending_names: Option<&HashSet<&str>>, queue: &mut VecDeque<(PathBuf, bool)>, ) { @@ -358,7 +360,7 @@ impl LegacyNpmCrawler { // BFS order guarantees a root-linked install has already been // probed (and removed from `pending`) before these are // dequeued, so a package is never resolved twice. - if name_str == ".pnpm" { + if !store_entry && name_str == ".pnpm" { let Some(file_type) = crate::utils::fs::entry_file_type(&entry).await else { continue; }; @@ -377,7 +379,7 @@ impl LegacyNpmCrawler { // vlt's store has the same transitive-only-home property: every // package lives at `.vlt//node_modules/` and the // importer holds only links into it. - if name_str == VLT_STORE_NAME { + if !store_entry && name_str == VLT_STORE_NAME { let Some(file_type) = crate::utils::fs::entry_file_type(&entry).await else { continue; }; @@ -394,7 +396,7 @@ impl LegacyNpmCrawler { // Must run before the generic hidden-entry skip below, which // would otherwise swallow it — leaving every transitive-only // install unpatchable on those layouts. - if is_legacy_pnpm_store_dir_name(&name_str) { + if !store_entry && is_legacy_pnpm_store_dir_name(&name_str) { let Some(file_type) = crate::utils::fs::entry_file_type(&entry).await else { continue; }; From ce351db6451ed448eb6d153ca0ec9b61d5d300f9 Mon Sep 17 00:00:00 2001 From: socket-patch-agent Date: Mon, 2 Nov 2026 13:35:11 +0000 Subject: [PATCH 4/5] Skip nested stores of every entry shape The peer fan-out's nested-store check only recognised a //node_modules path. A store nested in a scoped npm .store entry, a pnpm <=3 .registry.* entry or an entry of a store relocated by virtualStoreDir was still probed, so apply could patch a peer copy scan never reports. The fan-out now drops any found store that sits inside another found store or inside a store-named dir. Refs #1899 Assisted-by: Claude Code:claude-opus-5-5 --- .../src/crawlers/npm_crawler.rs | 84 +++++++++++++++---- 1 file changed, 66 insertions(+), 18 deletions(-) diff --git a/crates/socket-patch-core/src/crawlers/npm_crawler.rs b/crates/socket-patch-core/src/crawlers/npm_crawler.rs index 807ed1302..2ccd6030f 100644 --- a/crates/socket-patch-core/src/crawlers/npm_crawler.rs +++ b/crates/socket-patch-core/src/crawlers/npm_crawler.rs @@ -616,22 +616,26 @@ fn pnpm_shaped_store_layout(name: &str) -> Option { .map(|(_, layout)| *layout) } -/// Whether `dir` is a named store's entry's own `node_modules` -/// (`//node_modules`, `` one of the pnpm-shaped -/// stores, `.vlt` or `.store`), whose children are never stores (see -/// [`StoreKind::classify`]). -fn is_store_entry_node_modules(dir: &Path) -> bool { - dir.file_name() == Some(OsStr::new("node_modules")) - && dir - .parent() - .and_then(Path::parent) - .and_then(Path::file_name) - .and_then(OsStr::to_str) - .is_some_and(|store| { - pnpm_shaped_store_layout(store).is_some() - || store == VLT_STORE_NAME - || store == NPM_LINKED_STORE_NAME +/// Whether `store` sits inside another store: below one of `stores`, or +/// below a dir whose name makes it a store (pnpm-shaped, `.vlt`, pnpm +/// <=3's `.registry.*`, npm's `.store` in a `node_modules`). Such a +/// "store" lives in a store entry's own `node_modules`, which never holds +/// a store (see [`StoreKind::classify`]), so the peer fan-out skips it as +/// the scan and the resolver do. Scoped and legacy entries nest deeper +/// than `//node_modules`, and a store relocated by +/// `virtualStoreDir` has no fixed name, hence the walk up every ancestor. +fn is_inside_another_store(store: &Path, stores: &[(StoreLayout, PathBuf)]) -> bool { + store.ancestors().skip(1).any(|dir| { + stores.iter().any(|(_, other)| other == dir) + || dir.file_name().and_then(OsStr::to_str).is_some_and(|name| { + pnpm_shaped_store_layout(name).is_some() + || name == VLT_STORE_NAME + || is_legacy_pnpm_store_dir_name(name) + || (name == NPM_LINKED_STORE_NAME + && dir.parent().and_then(Path::file_name) + == Some(OsStr::new("node_modules"))) }) + }) } /// Which package-manager store a `node_modules` child is. The resolver @@ -4523,9 +4527,7 @@ async fn find_store_peer_variant_copies_reusing( stores.push((StoreLayout::NpmLinked, dir.to_path_buf())); } } - // A store entry's own `node_modules` never holds a store - // (see `StoreKind::classify`), so only other ones are probed. - Some("node_modules") if !is_store_entry_node_modules(dir) => { + Some("node_modules") => { let others = [ (VLT_STORE_NAME, StoreLayout::Vlt), (NPM_LINKED_STORE_NAME, StoreLayout::NpmLinked), @@ -4609,6 +4611,9 @@ async fn find_store_peer_variant_copies_reusing( } } } + // A store nested in another store's entry is not a store (#1899). + let all_stores = stores.clone(); + stores.retain(|(_, store)| !is_inside_another_store(store, &all_stores)); if stores.is_empty() { return Vec::new(); } @@ -6765,6 +6770,49 @@ mod tests { } } + /// #1899: apply's peer fan-out skips a store nested in a store entry's + /// own `node_modules`, whatever shape the outer entry has: a pnpm + /// entry, a scoped npm `.store` entry, a pnpm <=3 `.registry.*` entry + /// (both nest deeper than `//node_modules`) or an entry + /// of a store relocated by `virtualStoreDir`. + #[tokio::test] + async fn test_peer_fan_out_skips_a_store_nested_in_any_store_entry() { + for (outer_entry_nm, modules_yaml) in [ + ("node_modules/.pnpm/@s+outer@1.0.0/node_modules", None), + ( + "node_modules/.store/@s+outer@1.0.0-abcdef/node_modules", + None, + ), + ( + "node_modules/.registry.npmjs.org/@s/outer/1.0.0/node_modules", + None, + ), + ( + ".vs/@s+outer@1.0.0/node_modules", + Some("layoutVersion: 5\nvirtualStoreDir: '../.vs'\n"), + ), + ] { + let tmp = tempfile::tempdir().unwrap(); + let root: PathBuf = tmp.path().components().collect(); + let entry_nm = root.join(outer_entry_nm); + let outer = entry_nm.join("@s/outer"); + write_pkg(&outer, "@s/outer", "1.0.0"); + if let Some(yaml) = modules_yaml { + std::fs::create_dir_all(root.join("node_modules")).unwrap(); + std::fs::write(root.join("node_modules/.modules.yaml"), yaml).unwrap(); + } + let nested_twin = + entry_nm.join(".pnpm/@s+outer@1.0.0(react@18.2.0)/node_modules/@s/outer"); + write_pkg(&nested_twin, "@s/outer", "1.0.0"); + + let copies = find_store_peer_variant_copies(&outer).await; + assert!( + !copies.contains(&nested_twin), + "{outer_entry_nm}: peer fan-out walked a nested store: {copies:?}" + ); + } + } + /// #362: pnpm's `virtualStoreDir` moves the virtual store, and /// `node_modules/.modules.yaml` records where (relative to /// `node_modules`: JSON on pnpm 10+, YAML before). A relocated store From 712ce24352e2620e45fa6ab8a810111fef5ca3b5 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 16:08:49 +0000 Subject: [PATCH 5/5] Keep the peer fan-out's store when a store name sits above it is_inside_another_store matched store names on every ancestor, and the ancestor walk also collects any `.pnpm`/`.bun`/`.deno`/`.vlt` dir above the project into `stores`. A project under `~/.bun` (Bun's global install root), `~/.deno` or `~/.vlt` therefore had its own store dropped, so apply and rollback silently skipped peer-variant copies the scan still reports, the reverse of the #1899 disagreement. A fixed store name now counts only on a `node_modules` child, and the `stores` comparison only counts stores without a fixed name (those relocated by `virtualStoreDir`). A new test covers projects under `.deno`, `.vlt` and `.bun/install/global`; it fails without this change. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../src/crawlers/npm_crawler.rs | 68 ++++++++++++++----- 1 file changed, 51 insertions(+), 17 deletions(-) diff --git a/crates/socket-patch-core/src/crawlers/npm_crawler.rs b/crates/socket-patch-core/src/crawlers/npm_crawler.rs index 2ccd6030f..add60f1c8 100644 --- a/crates/socket-patch-core/src/crawlers/npm_crawler.rs +++ b/crates/socket-patch-core/src/crawlers/npm_crawler.rs @@ -616,25 +616,31 @@ fn pnpm_shaped_store_layout(name: &str) -> Option { .map(|(_, layout)| *layout) } -/// Whether `store` sits inside another store: below one of `stores`, or -/// below a dir whose name makes it a store (pnpm-shaped, `.vlt`, pnpm -/// <=3's `.registry.*`, npm's `.store` in a `node_modules`). Such a -/// "store" lives in a store entry's own `node_modules`, which never holds -/// a store (see [`StoreKind::classify`]), so the peer fan-out skips it as -/// the scan and the resolver do. Scoped and legacy entries nest deeper -/// than `//node_modules`, and a store relocated by -/// `virtualStoreDir` has no fixed name, hence the walk up every ancestor. +/// Whether `store` sits inside another store: below a `node_modules` +/// child whose name makes it a store (pnpm-shaped, `.vlt`, pnpm <=3's +/// `.registry.*`, npm's `.store`), or below one of `stores` that has no +/// such name (a store relocated by `virtualStoreDir`). Such a "store" +/// lives in a store entry's own `node_modules`, which never holds a store +/// (see [`StoreKind::classify`]), so the peer fan-out skips it as the scan +/// and the resolver do. A store-named dir that is not a `node_modules` +/// child (Bun's `~/.bun` install root, a project under `~/.deno`) is no +/// store, even though the ancestor walk collects it into `stores`, so it +/// never hides one. Scoped and legacy entries nest deeper than +/// `//node_modules`, and a relocated store has no fixed +/// name, hence the walk up every ancestor. fn is_inside_another_store(store: &Path, stores: &[(StoreLayout, PathBuf)]) -> bool { store.ancestors().skip(1).any(|dir| { - stores.iter().any(|(_, other)| other == dir) - || dir.file_name().and_then(OsStr::to_str).is_some_and(|name| { - pnpm_shaped_store_layout(name).is_some() - || name == VLT_STORE_NAME - || is_legacy_pnpm_store_dir_name(name) - || (name == NPM_LINKED_STORE_NAME - && dir.parent().and_then(Path::file_name) - == Some(OsStr::new("node_modules"))) - }) + let fixed_name = dir.file_name().and_then(OsStr::to_str).is_some_and(|name| { + pnpm_shaped_store_layout(name).is_some() + || name == VLT_STORE_NAME + || is_legacy_pnpm_store_dir_name(name) + || name == NPM_LINKED_STORE_NAME + }); + if fixed_name { + dir.parent().and_then(Path::file_name) == Some(OsStr::new("node_modules")) + } else { + stores.iter().any(|(_, other)| other == dir) + } }) } @@ -6813,6 +6819,34 @@ mod tests { } } + /// #1986 review: a store-named dir above the project (Bun's `~/.bun` + /// install root holding the global `node_modules`, a project under + /// `~/.deno` or `~/.vlt`) is not a `node_modules` child, so it is no + /// store and the peer fan-out still walks the project's own store. + #[tokio::test] + async fn test_peer_fan_out_keeps_a_store_below_a_store_named_ancestor() { + for project in [ + "home/plain/proj", + "home/.deno/proj", + "home/.vlt/proj", + ".bun/install/global", + ] { + let tmp = tempfile::tempdir().unwrap(); + let root: PathBuf = tmp.path().components().collect(); + let store = root.join(project).join("node_modules/.pnpm"); + let primary = store.join("a@1.0.0(react@17.0.2)/node_modules/a"); + let twin = store.join("a@1.0.0(react@18.2.0)/node_modules/a"); + write_pkg(&primary, "a", "1.0.0"); + write_pkg(&twin, "a", "1.0.0"); + + let copies = find_store_peer_variant_copies(&primary).await; + assert!( + copies.contains(&twin), + "{project}: peer fan-out dropped the project's store: {copies:?}" + ); + } + } + /// #362: pnpm's `virtualStoreDir` moves the virtual store, and /// `node_modules/.modules.yaml` records where (relative to /// `node_modules`: JSON on pnpm 10+, YAML before). A relocated store