[Mikola Lysenko (mikolalysenko)]

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

Refs #1899 (first slice: one store classifier and the store-entry rule across all three walks; see "Remaining slices").

Root cause

crawlers/npm_crawler.rs decided which node_modules child is a package-manager store (.pnpm/.bun/.deno, .vlt, npm's linked .store, .modules.yaml, pnpm <=3 .registry.*) in separate hand-written copies, and they drifted. The scan (gather_node_modules) never treated a store-named child of a store entry's own node_modules as a store. The resolver (nested_node_modules_of, used by apply, apply --check and vex) and apply's peer-variant fan-out (find_store_peer_variant_copies_reusing) did walk it. So apply could resolve and patch a copy scan never reported.

Fix

• New StoreKind enum with one StoreKind::classify(name, file_type, in_store_entry). The scan and the resolver both classify children through it, and the resolver now gets store_entry from visit_resolver_dir, as the scan does.
• The resolver's per-kind store listing moves into one resolver_store_entries.
• The peer-variant fan-out drops any store it finds inside another found store or inside a store-named dir (is_inside_another_store). That covers pnpm entries, scoped npm .store entries, pnpm <=3 .registry.* entries and entries of a store relocated by virtualStoreDir.
• The test-only oracle (npm_crawler/oracle.rs) follows the same store-entry rule, so it still matches the production resolver.
• No behaviour change outside store entries. Every store name starts with a dot, so a store name with the wrong file type still falls through to the hidden-entry skip.

Tests

test_store_named_child_of_a_store_entry_is_not_a_store nests a copy of hidden@1.0.0 under each of .pnpm, .bun, .deno, .store, .registry.npmjs.org, .vlt and a .modules.yaml-relocated store, all inside a pnpm store entry's node_modules. It also puts a peer variant of the entry's own package in a nested .pnpm. The test asserts that crawl_all doesn't report the copy, find_by_purls doesn't resolve it, and find_store_peer_variant_copies doesn't reach the peer variant.

Red without the fix:

• on main: .pnpm: resolver walked a nested store: {"pkg:npm/hidden@1.0.0": [... node_modules/.pnpm/outer@1.0.0/node_modules/.pnpm/hidden@1.0.0/node_modules/hidden]}
• with the peer gate removed: .pnpm: peer fan-out walked a nested store
• with the classifier's store-entry check removed (only the .vlt case, then only .vs): .vlt: scan reported a nested store's copy and .vs: scan reported a nested store's copy. The scan now shares the classifier, so dropping that check breaks the scan too.

Green with the fix. Local runs:

• cargo fmt --all -- --check: pass
• cargo clippy --workspace --all-features -- -D warnings: pass
• cargo test -p socket-patch-core --all-features --no-fail-fast: 6997 lib tests pass and every integration suite passes, including crawler_npm_e2e and the oracle equivalence tests. Four lib tests fail locally: copy_tree::relax_loop_must_not_traverse_symlinked_root, vlt_heal::an_unremovable_hidden_lock_keeps_every_store_entry, pypi_poetry::wire_write_failure_maps_error_and_leaves_lock_untouched and pypi_requirements::wire_failure_rolls_back_already_written_files. All four depend on read-only file modes (0o555), which root ignores, and this container runs as root. None of them touch code this PR changes. CI runs them unprivileged.
• I didn't run the full cargo test --workspace locally because the container's disk allowance ran out while building it. CI runs it.
• The npm/pypi/gem wrappers are unaffected; this change is internal to the core crawler.

Remaining slices (#1899 stays open)

• The peer finder still lacks the legacy pnpm stores and the pnpm global virtual store.
• One store-entry type (StoreEntryDir) instead of four shapes, decoding each name once.
• One package_children iterator for the ~7 package-dir filters and the 5 @scope/name expansions.

Review

• Bugbot (ef6b60b) said the peer gate missed deeper or relocated store-entry shapes. Confirmed and fixed in ce351db. New test test_peer_fan_out_skips_a_store_nested_in_any_store_entry fails without the fix and passes with it.
• /code-review high raised 6 findings and all 6 are fixed: peer fan-out gate, .vlt/.modules.yaml test cases, the misplaced Agent mode ignores pnpm's virtualStoreDir: transitive dependencies are reported package_not_installed with a custom virtualStoreDir or the global virtual store #362 doc, oracle drift, Option → Vec, and stale docs.

https://claude.ai/code/session_011f9aeRrMp15bL56hH58ghD

---

Note

Medium Risk
Touches core npm node_modules traversal for scan, resolve, and apply peer fan-out; wrong classification could miss packages or skip valid peer variants, but behavior outside store-entry trees is intentionally unchanged and coverage is extensive.

Overview
Fixes #1899, where scan, apply resolver, and peer-variant fan-out disagreed on whether a store-named directory under a store entry's own node_modules (e.g. .pnpm/outer@1.0.0/node_modules/.pnpm/...) counts as a real package-manager store. The resolver and fan-out could patch copies that crawl never reported.

The change introduces a shared StoreKind::classify (name, file type, in_store_entry) and routes both scan deferral and resolver probing through it, with listing logic consolidated in resolver_store_entries. is_inside_another_store drops stores nested inside another store (including relocated virtualStoreDir layouts) from peer fan-out, while store-named ancestors that are not node_modules children (e.g. Bun/Deno install roots) still allow the project's own store to be walked.

The test oracle mirrors the store_entry gate so it stays aligned with production. New tests cover nested-store rejection across store shapes and regression #1986 for ancestors that look like store names but are not stores.

^{Reviewed by Cursor Bugbot for commit 712ce24. Configure here.}

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

BugBot review

---

Generated by Claude Code

[cursor]

Stale Bugbot comment from a previous run.

[Mikola Lysenko (mikolalysenko)]

BugBot review

---

Generated by Claude Code

[cursor]

Stale Bugbot comment from a previous run.

[Mikola Lysenko (mikolalysenko)]

Burn-down agent: labeled Ready for review.

• Head: ce351db6451ed448eb6d153ca0ec9b61d5d300f9, no conflicts with main (cbbbdb9).
• CI: all 50 check runs on the head are success/skipped (ci-ok green). No main-wide failures.
• Bugbot: reviewed ce351db clean. Its one earlier finding (peer gate missing store-entry shapes, on ef6b60b) was fixed in ce351db and the thread is resolved.
• Reviewer focus: StoreKind::classify in crawlers/npm_crawler.rs is now shared by scan and resolver; check the is_inside_another_store filter on the peer-variant fan-out.
• Slack announcement not sent this run (Slack send unavailable); the next run will retry.

---

Generated by Claude Code

[Wenxin Jiang (Wenxin-Jiang)]

[agent] Reviewed ce351db6451ed448eb6d153ca0ec9b61d5d300f9. Checked unified store classification, resolver/scan traversal and the repaired scoped/relocated peer-fan-out gate. No major correctness issues found. Validation: source and regression-test review plus the available exact-head CI checks; no local test pass is claimed.

[Mikola Lysenko (mikolalysenko)]

Final reviewer: claim released.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Final review: one fix pushed (712ce24)

I found one blocking problem in the review and fixed it in 712ce24.

The problem: the peer fan-out dropped the project's own store when a store-named directory sat above the project.

• is_inside_another_store matched .pnpm/.bun/.deno/.vlt/.registry.* on every ancestor, including directories above the project root.
• The fan-out's ancestor walk also collects any such directory into stores.
• So for a project under ~/.bun (Bun's global install root, e.g. ~/.bun/install/global), ~/.deno or ~/.vlt, stores.retain(...) removed the real node_modules/.pnpm or node_modules/.bun.
• As a result, apply and rollback silently skipped peer-variant twins such as .pnpm/a@1.0.0(react@18.2.0)/node_modules/a, while scan still reported them. That is npm_crawler classifies node_modules store children three times, and the copies drifted on store entries and legacy/global stores #1899's scan/apply disagreement in the other direction.

The fix:

• A fixed store name now counts only on a node_modules child.
• The comparison against stores now only counts stores without a fixed name, i.e. stores relocated by virtualStoreDir.
• New test test_peer_fan_out_keeps_a_store_below_a_store_named_ancestor covers projects under .deno, .vlt and .bun/install/global. It fails on the previous head with dropped the project's store: [].

What I ran:

• cargo test -p socket-patch-core --lib npm_crawler: 108 passed.
• The full socket-patch-core lib suite: the same 4 failures as before my change. They are permission-based tests that fail because the container runs as root, and none of them are npm-related.
• cargo clippy -p socket-patch-core --lib -- -D warnings: clean.

I removed Ready for review so that CI and Bugbot run again on the new head. A review brief follows once it's green.

bugbot run

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

bugbot run

---

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 712ce24. Configure here.}

[Mikola Lysenko (mikolalysenko)]

Burn-down agent: labeled Ready for review.

• Head: 712ce24352e2620e45fa6ab8a810111fef5ca3b5 (final reviewer's fan-out store fix), mergeable with main.
• CI: 50/50 check runs success/skipped on the head.
• Bugbot: reviewed 712ce24, no new issues; the one review thread is resolved.
• Reviewer note: the earlier approval was on ce351db; 712ce24 (keep the peer fan-out's store when a store name sits above it) needs a fresh look.
• Slack announcement not sent: no Slack send tool in this run.

---

Generated by Claude Code