Fix npm crawler missing relocated dependency stores (#359, #362) by mikolalysenko · Pull Request #365 · SocketDev/socket-patch

Mikola Lysenko (mikolalysenko) · 2026-09-30T19:23:48Z

LLM Description written by Claude Code:claude-opus-5-5

Fixes #359
Refs #362

Summary

Agent mode now finds transitive npm packages in two install layouts it used to miss. Before this change, scan, apply, rollback and vendor reported those packages package_not_installed and left them unpatched, and scan --apply still exited 0.

npm install-strategy=linked (npm install-strategy=linked: transitive packages under node_modules/.store are "not installed", and scan --apply exits 0 leaving them unpatched #359): npm keeps every package in node_modules/.store/<name>@<version>-<hash>/node_modules/<name>. Scoped packages sit one level deeper, at .store/@scope/<leaf>@…. The crawler now walks that store in all three places that look for installed copies:
- the scan walk (gather_node_modules);
- apply/rollback's resolver (nested_node_modules_of);
- the peer-copy fan-out (find_store_peer_variant_copies). npm hashes the dependency graph into the key, so one name@version can have several copies.
It uses the existing store-entry policy: real dirs only, and links inside an entry are dependency edges. decode_npm_store_entry_name reads the advertised name and version. It cuts the hash by its fixed 22-char length, because the base64url hash can contain - itself.
pnpm virtualStoreDir (Agent mode ignores pnpm's virtualStoreDir: transitive dependencies are reported package_not_installed with a custom virtualStoreDir or the global virtual store #362, custom-dir half): pnpm records a relocated virtual store in node_modules/.modules.yaml. That's JSON on pnpm 10+ and YAML before, relative to node_modules, or absolute on old pnpm. relocated_pnpm_virtual_store_sync reads it, and the same three walks treat the recorded dir like .pnpm. That covers a store next to node_modules (.vstore) as well as one under a custom hidden name inside it.
Not walked on purpose: a recorded store outside the project, or one reached through a link. In practice that's pnpm's global virtual store (enableGlobalVirtualStore, <store-dir>/v10/links). Every project on the machine loads those files, so patching them in place would patch the other projects too, which is Agent-mode apply and rollback on a pnpm project with enableGlobalVirtualStore patch (and unpatch) every other project that shares the store #361's bug. The peer-copy fan-out also only uses a relocated store that holds the copy being patched, so an enclosing project's store is never touched.
- "Inside" means strictly below the importer by plain child names. That also holds when the importer is the empty path of the default --cwd . (9708b3d).
- An absolute recorded store is also compared with both sides canonicalized. So an in-project store is still found from --cwd . or through a linked ancestor (6c155c6).
- The default node_modules/.pnpm is skipped by its importer-relative tail, however it is spelled, so it is never walked twice (80f4a71).
The last three items came from Bugbot reviews.
Docs: docs/ecosystems.md ("npm: which node_modules trees are crawled") lists the stores that are walked and the global-store exclusion. CHANGELOG has a Fixed entry.

Root cause

npm_crawler.rs recognized isolated-layout stores only by fixed names: .pnpm, pnpm ≤3 .registry.*, and .vlt. Every other hidden node_modules child hit the generic hidden-entry skip. npm's .store and any relocated pnpm store are the only home of transitive dependencies, so those packages were invisible. Both issues come from that one name check, and one store-discovery change fixes both.

Scope

#366 (Bun node_modules/.bun), #373 (Deno node_modules/.deno) and #405 (hosted VEX attesting from the pin while the .bun copy is unpatched) share this root cause. They are not in this PR, to keep it reviewable.

.bun entry names need their own decoding: +<hash> peer suffixes, and hosted http+++… entries.
They can follow as one PR once this lands, reusing the same store-discovery hooks.

Refs #362, not Fixes. The global-virtual-store half of #362 is left for #361, because making <store>/links visible without per-project isolation would widen #361's cross-project patching. I've explained this on #362, which stays open for that part.

Merge with post-#277 `main` (`1eb28a7`)

The v5 consolidation (#277) rewrote CHANGELOG. I kept main's version and re-added this PR's Fixed bullet. I also pointed the .modules.yaml reader at strip_bom's new home, utils::serde, because package_json::detect was removed. No other conflicts.

Test evidence

Issue	Case	Regression test	Red without fix	Green
#359	transitive + scoped + two-hash copies in `.store`: scan, resolver, peer fan-out	`crawlers::npm_crawler::tests::test_npm_linked_store_transitive_packages_are_found`	✅ fails	✅
#359	store-key decoding (hash with `-`/`_`, prerelease, scoped, shrinkwrap dir)	`test_decode_npm_store_entry_name`	new helper	✅
#359	real npm, `install-strategy=linked`, is-odd → is-number: `apply` patches the copy Node loads, `rollback` restores it, `vendor` rewires the lock, `vendor --revert` is byte-exact	`e2e_vendor_npm_build::npm_linked_strategy_transitive_package_is_patched_rolled_back_and_vendored`	✅ fails (`apply failed`)	✅
#362	`virtualStoreDir` as JSON `../.vstore`, YAML absolute path, YAML `'.custom'`: scan, resolver, peer fan-out	`test_pnpm_relocated_virtual_store_dir_is_walked`	✅ fails	✅
#362	store outside the project, or reached through a link (relative or absolute record), is not walked	`test_pnpm_virtual_store_dir_outside_project_is_ignored`	guard	✅
#362	default `--cwd .`: an absolute outside store is not crawled; in-project stores, relative and absolute, are	`scan_pnpm_relocated_store_cwd_e2e` (3 tests)	✅ absolute in-project case fails on `1eb28a7`	✅
#362	absolute in-project store reached through a linked ancestor: scan and resolver	`test_absolute_in_project_store_is_walked_through_any_spelling`	✅ fails on `1eb28a7`	✅
#362	absolute recording of the default `.pnpm` / `node_modules` is not a relocated store (no double walk)	`test_absolute_default_store_is_not_a_relocated_store`	✅ fails on `6c155c6`	✅
#362	containment rule: only plain child names below the importer	`test_path_below_accepts_only_plain_children`	✅	✅
#362	an enclosing project's relocated store is not used for peer fan-out	`test_enclosing_projects_relocated_store_is_not_a_peer_variant_source`	guard	✅
#362	real pnpm 10, `virtualStoreDir: .vstore` and `node_modules/.custom`: `apply` patches the copy Node loads, `rollback` restores it	`e2e_vendor_pnpm_build::pnpm_agent_apply_patches_a_transitive_dep_in_a_relocated_virtual_store`	✅ fails	✅

Local results on 80f4a71, merged with main 2463257:

cargo test -p socket-patch-core --lib -- npm_crawler: 62 passed.
scan_pnpm_relocated_store_cwd_e2e: 3 passed.
The npm-linked and pnpm-relocated real-toolchain e2e tests (npm 10.9.4, pnpm 10.28.0): passed.
cargo clippy --workspace --all-features -- -D warnings: clean.
cargo fmt --all -- --check: the only diffs in touched files (visit_resolver_dir's signature, oracle.rs imports) are byte-identical on main. CI runs no fmt gate.
cargo test --workspace --all-features --no-fail-fast on 1eb28a7: 9,337 passed and 12 failed. All 12 are write-failure or permission tests that fail only because the sandbox runs as root, the same set reported on the other agent PRs. None are in crawler, npm or pnpm code.
The npm, PyPI and gem wrappers only dispatch, so they need no change.

CI on 80f4a71: all workflows are green (CI, npm, pnpm, Bun, vlt, Audit), and Bugbot reports no new issues. Earlier, on 6c155c6, the vlt legs failed between about 03:05 and 03:20 UTC. The cause was vlt ci itself getting 401 from api.socket.dev/v0/purl, which is outside this diff, and the vlt legs passed again after that window.

🤖 Generated with Claude Code

Generated by Claude Code

Assisted-by: Claude Code:claude-opus-5-5

With npm's install-strategy=linked, or a pnpm virtualStoreDir moved away from node_modules/.pnpm, transitive dependencies live only in a store the crawler never walked: it matched stores by fixed name and skipped every other hidden dir. apply, scan and vendor then reported those packages as not installed and left them unpatched. The crawler now walks npm's node_modules/.store (including scoped entries one level down) and the pnpm store that .modules.yaml records, in scan, apply's resolver and the peer-copy fan-out. A recorded store outside the project, such as pnpm's global virtual store shared by other projects, is still left alone. Assisted-by: Claude Code:claude-opus-5-5

Covers #359 (npm install-strategy=linked, transitive dep only in node_modules/.store: apply, rollback and vendor) and #362 (pnpm virtualStoreDir at .vstore and node_modules/.custom: apply and rollback), each checking that Node loads the patched copy. Both fail on main with package_not_installed. Assisted-by: Claude Code:claude-opus-5-5

Assisted-by: Claude Code:claude-opus-5-5

The fan-out that patches every peer-variant copy looked up a relocated pnpm store from any ancestor directory. For a project nested inside another pnpm project, that could pick the outer project's store and patch copies this project never loads. A relocated store now counts only when the copy being patched sits inside it. Assisted-by: Claude Code:claude-opus-5-5

npm 9.0-9.3 ignore install-strategy=linked and install the hoisted tree, so the npm 9.0.0 compatibility cell has no .store to test. Assisted-by: Claude Code:claude-opus-5-5

Mikola Lysenko (mikolalysenko) · 2026-09-30T20:12:00Z

BugBot review

Generated by Claude Code

cursor

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Autofix Details

Bugbot Autofix prepared a fix for the issue found in the latest run.

✅ Fixed: Relative cwd walks global pnpm store
- Added a check to reject empty importer paths, preventing strip_prefix from succeeding on absolute global store paths when cwd is '.'

Or push these changes by commenting:

@cursor push af6d4cb22e

Preview (af6d4cb22e)

diff --git a/crates/socket-patch-core/src/crawlers/npm_crawler.rs b/crates/socket-patch-core/src/crawlers/npm_crawler.rs
--- a/crates/socket-patch-core/src/crawlers/npm_crawler.rs
+++ b/crates/socket-patch-core/src/crawlers/npm_crawler.rs
@@ -310,6 +310,9 @@
     let text = crate::utils::fs::read_regular_to_string_sync(&nm.join(PNPM_MODULES_YAML)).ok()?;
     let recorded = parse_modules_yaml_virtual_store_dir(&text)?;
     let importer = normalize_lexically(nm.parent()?);
+    if importer.as_os_str().is_empty() {
+        return None;
+    }
     let store = normalize_lexically(&nm.join(recorded));
     if store == normalize_lexically(&nm.join(".pnpm")) || store == normalize_lexically(nm) {
         return None;
@@ -1088,7 +1091,11 @@
     /// Inside a store entry (`store_entry`) a link is a dependency edge into
     /// a sibling entry, whose own visit records that copy, so only a real
     /// directory there matches.
-    fn visit_resolver_dir(nm_path: PathBuf, store_entry: bool, pending: &[Target]) -> ResolverVisit {
+    fn visit_resolver_dir(
+        nm_path: PathBuf,
+        store_entry: bool,
+        pending: &[Target],
+    ) -> ResolverVisit {
         let listing = list_dir_sync(&nm_path);
         let probe_filter = ProbeFilter::new(&listing);
         let matched = pending

_{You can send follow-ups to the cloud agent here.}

Mikola Lysenko (mikolalysenko) · 2026-09-30T20:24:51Z

[agent] Two new issues share this PR's root cause (a dependency store dir that isn't on the hard-coded list gets dropped by the hidden-entry skip): #366 (Bun isolated linker, node_modules/.bun) and #373 (Deno nodeModulesDir, node_modules/.deno). I haven't added them to this PR's scope because another run holds a fresh heartbeat on it. Whoever next drives this PR can pick them up, or they can follow once it lands.

Generated by Claude Code

On macOS the temp dir sits under the /var -> /private/var link, so the peer-variant fan-out from a direct dep's importer link only matched the relocated store on the canonical chain and reported the copy as /private/var/..., unlike the other store layouts. Containment is now also checked canonically, so the store is kept as the caller spelled it. The regression test reaches its project through a linked ancestor so this is covered on every platform. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01HcXxpY7X3MC84EEfA8jws6

With the CLI's default `--cwd .` the importer is the empty relative path, and `strip_prefix("")` accepts every path, so an absolute `virtualStoreDir` such as pnpm's global virtual store passed the in-project check and was crawled (and would be patched in place). The store must now sit strictly below the importer by plain child names: no root, prefix or `..` component. Reported by Cursor Bugbot on #365. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01HcXxpY7X3MC84EEfA8jws6

Mikola Lysenko (mikolalysenko) · 2026-09-30T20:41:12Z

BugBot review

Generated by Claude Code

cursor

Stale Bugbot comment from a previous run.

The Windows test helper shells out to `cmd /C mklink /J`, which reads a `/` inside a path (`[email protected]/node_modules/is-number`) as a switch, so the linked-store and relocated-store tests failed on Windows with "Invalid switch". Rebuild both paths from their components first so every separator is `\`. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01HcXxpY7X3MC84EEfA8jws6

Mikola Lysenko (mikolalysenko) · 2026-09-30T22:15:03Z

[agent] test (windows-latest) on 0c511ec shows as cancelled, not failed. It hit the job's 50-minute timeout-minutes (21:23:09 → 22:13:17), 7 seconds after the last doc-test finished. Every test result in the log is ok, with zero FAILED. That includes the three tests this PR adds or changes that run there (test_npm_linked_store_transitive_packages_are_found, test_pnpm_relocated_virtual_store_dir_is_walked, test_path_below_accepts_only_plain_children); they now pass on Windows after the mklink separator fix. The same job took 38 minutes on 9708b3d. This PR adds only a handful of fast tests, so the extra 12 minutes looks like runner speed rather than this change. I'm re-running the failed jobs once. If it times out again, I'll treat it as this PR's problem and profile which binaries got slower.

Generated by Claude Code

Mikola Lysenko (mikolalysenko) · 2026-10-01T00:23:19Z

BugBot review

Generated by Claude Code

cursor

Stale Bugbot comment from a previous run.

Brings in the v5 consolidation (#277). Keeps main's rewritten CHANGELOG and re-adds this branch's Fixed entry, and points the virtualStoreDir reader at strip_bom's new home in utils::serde. Assisted-by: Claude Code:claude-opus-5-5

Mikola Lysenko (mikolalysenko) · 2026-10-01T02:37:42Z

BugBot review

Generated by Claude Code

cursor

Stale Bugbot comment from a previous run.

Old pnpm records virtualStoreDir as an absolute path. With the default --cwd . the importer is the empty path, which is never a prefix of an absolute store, so a store inside the project was skipped and its transitive packages stayed package_not_installed. An absolute store is now also compared with both sides canonicalized, which also covers a project reached through a linked ancestor. A store that resolves outside the project, such as pnpm's global virtual store, is still refused. Reported by Cursor Bugbot on #365. Assisted-by: Claude Code:claude-opus-5-5

Mikola Lysenko (mikolalysenko) · 2026-10-01T02:59:26Z

BugBot review

Generated by Claude Code

cursor

Stale Bugbot comment from a previous run.

pnpm's default store, node_modules/.pnpm, is walked by name. An old or Windows pnpm can record it in .modules.yaml as an absolute path, and from --cwd . (or through a linked ancestor) that spelling missed the lexical default-location check. The relocated-store reader then accepted it too, so every store copy was reported twice. The check now compares the importer-relative tail, so any spelling of the default store, or of node_modules itself, is skipped. Reported by Cursor Bugbot on #365. Assisted-by: Claude Code:claude-opus-5-5

Mikola Lysenko (mikolalysenko) · 2026-10-01T03:47:40Z

BugBot review

Generated by Claude Code

cursor

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit 80f4a71. Configure here.}

Mikola Lysenko (mikolalysenko) · 2026-10-01T05:12:57Z

[burn-down agent] Ready for review on 80f4a71: mergeable, 0 commits behind main. CI: 387/387 completed checks green (6 workflow skips, no failures). Bugbot reviewed 80f4a71 and found no new issues; no unresolved review threads. Bugbot findings from earlier rounds (importer-relative containment, canonicalized absolute store, default .pnpm dedupe) were fixed in 9708b3d/6c155c6/80f4a71; reviewer should look at the global-virtual-store exclusion in npm_crawler.rs.

Generated by Claude Code

Start fix for #359, #362

ce04c66

Assisted-by: Claude Code:claude-opus-5-5

Claude (claude) added 5 commits September 30, 2026 19:40

Document npm .store and pnpm virtualStoreDir walks

6e84b6d

Assisted-by: Claude Code:claude-opus-5-5

Skip the linked-store e2e below npm 9.4

4e88251

npm 9.0-9.3 ignore install-strategy=linked and install the hoisted tree, so the npm 9.0.0 compatibility cell has no .store to test. Assisted-by: Claude Code:claude-opus-5-5

Mikola Lysenko (mikolalysenko) marked this pull request as ready for review September 30, 2026 20:11

cursor Bot reviewed Sep 30, 2026

View reviewed changes

Comment thread crates/socket-patch-core/src/crawlers/npm_crawler.rs

Claude (claude) added 2 commits September 30, 2026 20:27

cursor Bot reviewed Sep 30, 2026

View reviewed changes

cursor Bot reviewed Oct 1, 2026

View reviewed changes

Mikola Lysenko (mikolalysenko) mentioned this pull request Oct 1, 2026

With Bun's isolated linker, vex attests a hosted patch as not_affected (verified) while the installed copy under node_modules/.bun is still unpatched (v5 regression) #405

Open

Merge main into agent/fix-npm-crawler-store-discovery

1eb28a7

Brings in the v5 consolidation (#277). Keeps main's rewritten CHANGELOG and re-adds this branch's Fixed entry, and points the virtualStoreDir reader at strip_bom's new home in utils::serde. Assisted-by: Claude Code:claude-opus-5-5

cursor Bot reviewed Oct 1, 2026

View reviewed changes

Comment thread crates/socket-patch-core/src/crawlers/npm_crawler.rs Outdated

cursor Bot reviewed Oct 1, 2026

View reviewed changes

Comment thread crates/socket-patch-core/src/crawlers/npm_crawler.rs Outdated

cursor Bot reviewed Oct 1, 2026

View reviewed changes

Mikola Lysenko (mikolalysenko) added the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label Oct 1, 2026

This was referenced Oct 1, 2026

Global agent mode on pnpm 12 (and 11 without the global virtual store) patches only one of the per-install copies of a package, reports success, and VEX attests not_affected #435

Open

Fix vlt e2e legs failing when api.socket.dev blips #448

Open

Conversation

Mikola Lysenko (mikolalysenko) commented Sep 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

LLM Description written by Claude Code:claude-opus-5-5

Summary

Root cause

Scope

Merge with post-#277 main (1eb28a7)

Test evidence

Uh oh!

Mikola Lysenko (mikolalysenko) commented Sep 30, 2026

Uh oh!

cursor Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Mikola Lysenko (mikolalysenko) commented Sep 30, 2026

Uh oh!

Mikola Lysenko (mikolalysenko) commented Sep 30, 2026

Uh oh!

cursor Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mikola Lysenko (mikolalysenko) commented Sep 30, 2026

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 1, 2026

Uh oh!

cursor Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 1, 2026

Uh oh!

cursor Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 1, 2026

Uh oh!

cursor Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 1, 2026

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 1, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Mikola Lysenko (mikolalysenko) commented Sep 30, 2026 •

edited

Loading

Merge with post-#277 `main` (`1eb28a7`)

cursor Bot left a comment •

edited

Loading

cursor Bot left a comment •

edited

Loading

cursor Bot left a comment •

edited

Loading

cursor Bot left a comment •

edited

Loading

cursor Bot left a comment •

edited

Loading