Resolve pnpm modules dirs through one helper for crawler and layout detection (#1129)

[Mikola Lysenko (mikolalysenko)]

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

Fixes #1129

Summary

"Where does pnpm keep this project's install?" now has one answer: crawlers::pnpm_layout. Before this PR there were two. The npm crawler honored pnpm's modulesDir, but detect_npm_pkg_manager and the pnpm Plug'n'Play carve-out (pnpm_pnp_layout_in) checked only the literal node_modules/. So a node-linker=pnp project with modulesDir: deps was read as yarn berry:

• agent apply refused it with yarn_pnp_unsupported and told the user to run yarn patch;
• vendored mode refused it with the yarn remedy;
• VEX read pnpm's loader as yarn's.

Why

Leverage: B 1 (#1129), U 0, D 1 (two implementations of pnpm's install location become one), R M. Score 3, P2. This was the top free candidate in the refactor register. The other candidates are blocked by file overlap with open PRs. Register row E92 is in register/10-audit-ecosystems.md, and the living-document passage is in doc/06-discovery-vex.md.

What changed

• New crawlers/pnpm_layout.rs:
  • configured_modules_dirs(project) is the crawler's former pnpm_modules_dirs. It resolves the modulesDir setting and finds child dirs that hold .modules.yaml. It no longer lists a dir twice when that dir is both configured and holds the record.
  • installed_store_in(view) / installed_store(project) check for .modules.yaml or .pnpm/ in node_modules/ first, and then in the configured dirs.
  • The view variant works on disk, on a snapshot, and in memory. On a snapshot it reads through root(), like yarn_node_linker, so a recording that reaches it is not reused. In memory, only the project's own pnpm-workspace.yaml and .npmrc count.
• npm_crawler.rs:
  • pnpm_modules_dirs, pnpm_modules_dir_setting and unquote_yaml_scalar are moved out of this file.
  • PNPM_MODULES_YAML is now the shared constant.
  • resolve_modules_folder becomes pub(super) so the new module can share it.
• pkg_managers.rs: step 4 of detect_npm_pkg_manager and pnpm_pnp_layout_in call pnpm_layout. The hard-coded node_modules/.modules.yaml / node_modules/.pnpm probes are deleted. The carve-out now checks the cheap lockfile conditions before it looks for the store.

Deleted / diff

Production: +191 / −92 (the new module's production half is 171 lines, about 60 of them doc comments). Tests: +213 (the pnpm_layout tests plus 1 CLI test).

 crates/socket-patch-cli/tests/e2e_safety_yarn_pnp.rs |  44 +++
 crates/socket-patch-core/src/crawlers/mod.rs         |   1 +
 crates/socket-patch-core/src/crawlers/npm_crawler.rs |  92 +-----
 crates/socket-patch-core/src/crawlers/pkg_managers.rs|  19 +-
 crates/socket-patch-core/src/crawlers/pnpm_layout.rs | 340 ++++++ (169 tests)

Behavior

• A project whose pnpm store is in a configured modulesDir (or in a child dir holding .modules.yaml) is detected as pnpm:
  • with a PnP loader, apply patches the crawled copies instead of refusing; vendored mode refuses with vendor_pnpm_pnp_unsupported and the pnpm remedy; and YarnPnpLoader::detect returns None;
  • without a PnP loader, apply prints the pnpm copy-on-write note instead of nothing.
• Every other layout behaves as before. The crawler's roots are unchanged, and configured_install_roots already deduplicated them.

Test evidence

• Red → green:
  • e2e_safety_yarn_pnp::pnpm_pnp_with_a_custom_modules_dir_applies (both the .npmrc and the pnpm-workspace.yaml spelling). With main's src/ it fails: error.code = yarn_pnp_unsupported, exit 1. On the branch it passes: exit 0 and deps/dummy/index.js is patched.
  • pnpm_layout::tests::every_reader_finds_a_store_in_the_configured_modules_dir runs every former reader on 4 layouts and checks each answer: crawler roots, detect_npm_pkg_manager, pnpm_pnp_layout (disk, memory, snapshot), YarnPnpLoader::detect and detect_npm_lock_flavor. The 4 layouts are .npmrc, pnpm-workspace.yaml plain and quoted, and an unconfigured dir holding .modules.yaml.
  • Fail-closed cases: no store in the configured dir, or a yarn.lock next to the loader, still detect yarn berry.
• cargo test -p socket-patch-core --lib -- crawlers:: vendor::npm_flavor vendor::lock_inventory: 890 passed.
• cargo test -p socket-patch-cli --all-features --test e2e_safety_yarn_pnp: 41 passed.
• cargo clippy --workspace --all-features -- -D warnings: clean.
• cargo test -p socket-patch-core --lib: 6095 passed, 4 failed. The 4 are the known root-sandbox failures that also fail on main (relax_loop_must_not_traverse_symlinked_root, an_unremovable_hidden_lock_keeps_every_store_entry, wire_write_failure_maps_error_and_leaves_lock_untouched, wire_failure_rolls_back_already_written_files) and pass in CI. cargo test -p socket-patch-core --test '*': all passed. spawn_env_hygiene: 12 passed.

Risk

Medium-low. The change is limited to layout detection. detect_npm_pkg_manager now reads pnpm-workspace.yaml/.npmrc from the project upward, plus one listing of the project root, but only when node_modules/ holds no pnpm store. That happens once per apply/vex, not per package.

🤖 Generated with Claude Code

https://claude.ai/code/session_019GNMsU2GoB5YNpiTvAX7ht

---

Note

Medium Risk
Changes package-manager and PnP classification paths used by apply, scan, and vendored mode; behavior is narrowed to custom pnpm modulesDir layouts with broad unit/e2e coverage.

Overview
Fixes #1129 by giving the npm crawler and package-manager layout detection a single source of truth for where pnpm installs packages (crawlers::pnpm_layout).

Before: only the crawler respected pnpm’s modulesDir (and stores under <modulesDir>/.pnpm). detect_npm_pkg_manager and the pnpm-vs-yarn PnP carve-out looked only under node_modules/, so node-linker=pnp + custom modulesDir (e.g. deps/) was treated as yarn berry — apply returned yarn_pnp_unsupported instead of patching.

After: configured_modules_dirs, installed_store / installed_store_in (disk, snapshot, memory) are shared. pkg_managers step 4 and pnpm_pnp_layout_in use them; duplicate logic is removed from npm_crawler. pnpm PnP with a custom modules dir is classified as pnpm, crawled, and patched.

Tests add unit coverage in pnpm_layout and e2e pnpm_pnp_with_a_custom_modules_dir_applies for .npmrc and pnpm-workspace.yaml.

^{Reviewed by Cursor Bugbot for commit 2e2715b. Configure here.}

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

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 2e2715b. Configure here.}

[Mikola Lysenko (mikolalysenko)]

[agent] CI: e2e (ubuntu-latest, e2e_safety_pnpm) (and so ci-ok) is red, but the cause is outside this PR. The suite installs live minimist@1.2.2 and applies Socket patch 80630680-…, which was republished (#1293). apply finds no matching patch, so all 3 tests that expect index.js patched fail on the hash check (311f1e… vs 043f04…), and so does the layout-note test. This PR doesn't change what apply does in a standard pnpm tree: the node_modules/.modules.yaml probe still answers first. The fix is in #1301 (repins those suites). I haven't ported it here, because #1301 changes e2e_safety_pnpm.rs and the routine doesn't overlap open PRs' files. Once #1301 lands, merging main here should turn the leg green. Every other check passed. Bugbot left no inline findings.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

[agent] Correction to my comment above: hosted-e2e is red too, for the same reason. 8 of the e2e_hosted_production tests fail because the live redirect now resolves minimist@1.2.2 to the republished patch 642d7f02-… instead of the pinned 80630680-… (#1293). This PR touches no hosted code. #1301 repins e2e_hosted_production.rs as well, so merging main after it lands should clear both legs.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Ready for review at 16883078e6b2.

• CI: required checks ci-ok and clippy green; 8 check suites succeeded. 1 superseded workflow run(s) show as cancelled; the required gates passed on this head.
• Mergeable against main, no CHANGELOG.md change.
• Bugbot reviewed this head; no unresolved review threads.

Labeled Ready for review by the burn-down agent. Slack announcement pending (connector unavailable this run).

---

Generated by Claude Code