GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — Fix npm crawler missing configured install roots (#493, #518) by mikolalysenko · Pull Request #520 · SocketDev/socket-patch

Conversation

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Collaborator

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

Fixes #493
Fixes #518

Summary

The npm crawler now also looks where the package manager is configured to install, not just in directories named node_modules:

yarn classic --modules-folder (Agent mode ignores yarn classic's --modules-folder: packages installed there are reported "not installed" and scan --mode agent exits 0 leaving them unpatched #493): the effective modules folder from the .yarnrc files at or above the project, resolved the way yarn 1.x resolves it. Each key comes from the nearest .yarnrc that sets it, and its value is resolved against that file's directory. --install.modules-folder wins over --modules-folder. Only a result strictly inside the project is honored.
Rush (npm crawler never looks in Rush's common/temp store: hosted vex attests a transitive dep not_affected while its installed copy is unpatched, and agent apply reports it package_not_installed #518): common/temp/node_modules when rush.json is at the root. Rush runs pnpm there, so every transitive dependency lives in that .pnpm store, and the projects' node_modules only link their direct deps.

For users this means agent-mode apply/scan now patches packages in these layouts instead of reporting them package_not_installed. It also means vex no longer issues a false not_affected: it used to read the uncrawled, unpatched installed copy as "nothing installed" and attest from the hosted lock pin. Now installed evidence wins (hash_mismatch), as on a default layout. The vendored vendored_tree_out_of_sync disclosure also fires for a modules-folder tree.

Root cause

NpmCrawler::find_local_node_modules_dirs (crates/socket-patch-core/src/crawlers/npm_crawler.rs) built the install roots only from a walk that collects directories literally named node_modules and prunes temp (SKIP_DIRS). It never consulted the package manager's own install-location config. Every consumer goes through get_node_modules_paths: apply, scan, vex verification, vex_consumed's alias walk, and the vendored out-of-sync check. So that one function caused all the reported symptoms.

Change

configured_install_roots(start_path): the configured roots listed above.
merge_configured_install_roots: appends those roots to the walk's roots. Walked roots inside a configured root are dropped (the walk descends into a non-node_modules modules folder and would otherwise report deps/<pkg>/node_modules as a workspace), and duplicates are skipped. Walk order is otherwise unchanged, so the seen dedup winners on existing layouts don't change.
A small .yarnrc (yarn lockfile syntax) reader for --modules-folder and --install.modules-folder, kept apart: bare or quoted key and value, optional :, # comments, BOM/CRLF, last setting wins per key. The file is read through the regular-file guard. resolve_modules_folder resolves the value against the defining .yarnrc's directory and keeps it only if it is strictly inside the project, like composer's config.vendor-dir gate. Absolute, drive-qualified or escaping values are ignored, so crawl and apply stay inside the project.
The test-only equivalence oracle (npm_crawler/oracle.rs) uses the same helper, so the randomized walk-equivalence tests still compare like with like.

Out of scope (not reported in either issue): Rush subspaces' per-subspace temp folders, and a .yarnrc in the home or /etc directory that isn't above the project. Wrappers under npm/, pypi/, gem/ only dispatch to the binary and need no change.

Tests (red without the fix → green with it)

Issue	Test	Without fix
#493	`npm_crawler::tests::test_yarnrc_modules_folder_is_a_crawl_root`	roots `[deps/outer/node_modules]`, not `[deps]`
#493	`npm_crawler::tests::test_parse_yarnrc_modules_folder`	(new parser)
#493	`e2e_vex_redirect::yarn_modules_folder_install_is_hash_verified_not_lockfile_attested`	exit 0, `verified` / `not_affected` over the unpatched `deps/` copy
#493	`e2e_vex_vendor::vendored_modules_folder_tree_out_of_sync_warns`	no `vendored_tree_out_of_sync` warning
#493	`in_process_alternate_installers::yarn_modules_folder_install_is_patched`	apply exit 1
#493	`in_process_alternate_installers::yarn_classic_modules_folder_install_then_apply_patches_file` (real `yarn install` 1.22.22)	apply exit 1
#518	`npm_crawler::tests::test_rush_common_temp_is_a_crawl_root`	roots lack `common/temp/node_modules`
#518	`e2e_vex_redirect::rush_common_temp_install_is_hash_verified_not_lockfile_attested`	exit 0, `verified` / `not_affected` over the unpatched store copy
#518	`in_process_alternate_installers::rush_transitive_dep_in_common_temp_store_is_patched`	apply exit 1
#518	`e2e_redirect_rush_sim` tier 1 (real `pnpm@9 install --frozen-lockfile`): new stale-store leg	`rush, unpatched store copy: exit Some(0)`
both	`npm_crawler::tests::test_merge_configured_install_roots`	(new helper)
review	`test_resolve_modules_folder`, `test_escaping_modules_folder_is_not_a_crawl_root`	escaping `../outside` / absolute dir crawled
review	`test_fifo_yarnrc_does_not_block_the_crawl`	(would block on `open(2)`)
review	`test_inherited_yarnrc_modules_folder_resolves_against_its_dir`	`[]` (looked in `project/project/deps`)
review	`test_install_scoped_modules_folder_takes_precedence`	`[general]`, not `[specific]`

Commands run locally (Linux):

cargo clippy --workspace --all-features -- -D warnings (the CI command): clean.
cargo test -p socket-patch-core --lib: all 71 crawler tests pass. Four tests elsewhere fail only because this container runs as root (read-only-directory tests can't fail a write as uid 0): copy_tree::relax_loop_must_not_traverse_symlinked_root, vlt_heal::an_unremovable_hidden_lock_keeps_every_store_entry, pypi_poetry::wire_write_failure_…, pypi_requirements::wire_failure_rolls_back_…. This PR doesn't touch them, and they pass in CI.
cargo test -p socket-patch-cli --all-features --test e2e_vex_redirect (27/27), --test e2e_vex_vendor (23/23), --test in_process_alternate_installers (21/21), --test e2e_redirect_rush_sim -- --ignored (3/3, real pnpm).
Real yarn install --offline 1.22.22 confirms the inherited and install-scoped layouts: installs land in project/deps, project/specific and project/specific.
A full local cargo test --workspace --all-features build exceeds this container's disk allowance, so the full matrix runs in CI.
cargo fmt --all -- --check isn't a CI gate and isn't clean on main. Only this PR's hunks were formatted; no unrelated reformatting is included.

Review and CI

Bugbot (549f8c0) found 2 real issues, both fixed in 113ae51 with regression tests (each thread replied to and resolved):
- A .yarnrc FIFO could block the crawl. It's now read through the regular-file guard.
- An unguarded --modules-folder path could escape the project. It's now normalized and gated like composer vendor-dir.
Bugbot found no new issues on 113ae51.
Human review (113ae51) found 2 P2 issues, both fixed in ec95949 and verified against real yarn 1.22.22 installs:
- An inherited .yarnrc modules folder must resolve against its own directory.
- --install.modules-folder takes precedence over --modules-folder.
CI on 113ae51 was all green. native (ubuntu-latest, 0.0.0-16) (vlt) and native (ubuntu-latest, 1.3.10) (Bun) failed once during a production patch-API outage (Connection reset by peer, 02:28–02:35 UTC). Each passed on its single re-run.

🤖 Generated with Claude Code

https://claude.ai/code/session_012pqLNDF3U1KUabGfPsxoZ8

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          Start fix for #493, #518

0fa7f0a

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

This was referenced Oct 2, 2026

Agent mode ignores yarn classic's --modules-folder: packages installed there are reported "not installed" and scan --mode agent exits 0 leaving them unpatched #493

Closed

npm crawler never looks in Rush's common/temp store: hosted vex attests a transitive dep not_affected while its installed copy is unpatched, and agent apply reports it package_not_installed #518

Closed

Claude (claude) added 2 commits

October 2, 2026 01:45


          Crawl yarn modules-folder and Rush install roots

e1e9637

The npm crawler found a project's installed packages only in dirs
named node_modules, so it never looked where yarn classic installs
with `--modules-folder` or where Rush installs (common/temp). Agent
mode reported those packages as not installed and left them
unpatched, and hosted vex treated the unpatched copy as absent and
attested the patch from the lockfile pin alone.

The crawler now also reads the nearest .yarnrc --modules-folder and
adds common/temp/node_modules in a Rush repo (rush.json at the root).

Fixes #493, #518

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


          Add real yarn and Rush regression legs

549f8c0

A real yarn classic install with --modules-folder is patched by agent
apply, and on a real pnpm-installed Rush repo vex now refuses to
attest when the installed copy under common/temp is unpatched.

Refs #493, #518

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

Mikola Lysenko (mikolalysenko) marked this pull request as ready for review

October 2, 2026 02:11

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026

Copy link

Copy Markdown

Collaborator Author

BugBot review

Generated by Claude Code

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

cursor Bot reviewed

Oct 2, 2026

View reviewed changes

cursor Bot left a comment •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.

Autofix Details

Bugbot Autofix prepared fixes for both issues found in the latest run.

✅ Fixed: Bare read of workspace .yarnrc
- Replaced bare std::fs::read_to_string with read_regular_to_string_sync to prevent blocking on FIFO files at the .yarnrc path.
✅ Fixed: Unguarded yarn modules-folder path
- Added normalize_modules_folder function to normalize paths and applied is_safe_multi_segment check to prevent path traversal and escaping project root.

Or push these changes by commenting:

@cursor push 515f68cd5e

Preview (515f68cd5e)

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
@@ -8,7 +8,7 @@
 use super::types::{CrawledPackage, CrawlerOptions};
 use super::walk_pool::{par_map, run_walk};
 use crate::patch::path_safety;
-use crate::utils::fs::{is_dir, is_dir_sync, read_dir_entries_sync};
+use crate::utils::fs::{is_dir, is_dir_sync, read_dir_entries_sync, read_regular_to_string_sync};
 use crate::utils::purl::{percent_decode_purl_component, strip_purl_qualifiers};
 use crate::vendor::vlt_lock_text::decode_vlt_dep_id;
 
@@ -46,7 +46,11 @@
 pub(super) fn configured_install_roots(start_path: &Path) -> Vec<PathBuf> {
     let mut roots = Vec::new();
     if let Some(folder) = yarnrc_modules_folder(start_path) {
-        roots.push(start_path.join(folder));
+        if let Some(normalized) = normalize_modules_folder(&folder) {
+            if path_safety::is_safe_multi_segment(&normalized) {
+                roots.push(start_path.join(normalized));
+            }
+        }
     }
     if start_path.join("rush.json").is_file() {
         roots.push(start_path.join("common").join("temp").join("node_modules"));
@@ -78,10 +82,12 @@
 }
 
 /// The `--modules-folder` value from the nearest `.yarnrc` at or above
-/// `start_path`, if any `.yarnrc` sets it.
+/// `start_path`, if any `.yarnrc` sets it. Read `.yarnrc` via
+/// `read_regular_to_string_sync` for the same reason `package.json` is:
+/// a FIFO planted at that workspace path would wedge a plain read forever.
 fn yarnrc_modules_folder(start_path: &Path) -> Option<String> {
     start_path.ancestors().find_map(|dir| {
-        let rc = std::fs::read_to_string(dir.join(".yarnrc")).ok()?;
+        let rc = read_regular_to_string_sync(&dir.join(".yarnrc")).ok()?;
         parse_yarnrc_modules_folder(&rc)
     })
 }
@@ -137,6 +143,29 @@
     (end > 0).then(|| (s[..end].to_string(), &s[end..]))
 }
 
+/// Reduce a `--modules-folder` value to plain `a/b` segments before the
+/// safety gate. Yarn accepts `./`-prefixed and `.`-interleaved values
+/// (`./deps`, `lib/./deps`) and either separator, so those shapes must
+/// resolve rather than be refused. `..` is resolved lexically the same way
+/// Composer's config.vendor-dir normalization does; a value that climbs
+/// above the project root (or reduces to it) fails closed as `None`.
+fn normalize_modules_folder(raw: &str) -> Option<String> {
+    if raw.starts_with(['/', '\\']) {
+        return None;
+    }
+    let mut segments: Vec<&str> = Vec::new();
+    for segment in raw.split(['/', '\\']) {
+        match segment {
+            "" | "." => {}
+            ".." => {
+                segments.pop()?;
+            }
+            other => segments.push(other),
+        }
+    }
+    (!segments.is_empty()).then(|| segments.join("/"))
+}
+
 // ---------------------------------------------------------------------------
 // Helper: read and parse package.json
 // ---------------------------------------------------------------------------
@@ -1237,7 +1266,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
@@ -4329,6 +4362,30 @@
         }
     }
 
+    #[test]
+    fn test_normalize_modules_folder() {
+        let n = normalize_modules_folder;
+        // Yarn-legal `./` prefixes and `.` segments reduce to the
+        // plain path; either separator is accepted.
+        assert_eq!(n("./deps").as_deref(), Some("deps"));
+        assert_eq!(n("./lib/deps").as_deref(), Some("lib/deps"));
+        assert_eq!(n("lib/./deps").as_deref(), Some("lib/deps"));
+        assert_eq!(n("lib\\deps").as_deref(), Some("lib/deps"));
+        assert_eq!(n("lib/../deps").as_deref(), Some("deps"));
+        assert_eq!(n("deps").as_deref(), Some("deps"));
+        // Escaping the project, reducing to it, or absolute — fail closed.
+        assert_eq!(n(".."), None);
+        assert_eq!(n("../elsewhere"), None);
+        assert_eq!(n("lib/../.."), None);
+        assert_eq!(n("."), None);
+        assert_eq!(n("a/.."), None);
+        assert_eq!(n("/etc/deps"), None);
+        assert_eq!(n("\\\\share\\deps"), None);
+        // A drive-letter segment survives normalization; the
+        // `is_safe_multi_segment` gate downstream rejects the colon.
+        assert_eq!(n("C:\\deps").as_deref(), Some("C:/deps"));
+    }
+
     fn local_options(cwd: &Path) -> CrawlerOptions {
         CrawlerOptions {
             cwd: cwd.to_path_buf(),

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

crates/socket-patch-core/src/crawlers/npm_crawler.rs Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

crates/socket-patch-core/src/crawlers/npm_crawler.rs Resolved

Uh oh!

There was an error while loading. Please reload this page.


          Keep yarn modules-folder inside the project

113ae51

The .yarnrc --modules-folder value comes from the project being
scanned and names a tree apply writes patches into. An absolute or
escaping value (/etc, ../elsewhere) is now ignored, like composer's
vendor-dir, so the crawl and apply stay inside the project. The
.yarnrc is also read with the regular-file guard, so a FIFO planted
at that path can no longer hang scan, apply or vex.

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

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026

Copy link

Copy Markdown

Collaborator Author

BugBot review

Generated by Claude Code

cursor[bot] reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

cursor Bot reviewed

Oct 2, 2026

View reviewed changes

cursor Bot left a comment •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026

Copy link

Copy Markdown

Collaborator Author

[agent] native (ubuntu-latest, 0.0.0-16) (vlt patch compatibility) failed on 113ae51. It doesn't look like this PR's failure:

The failing case is 0.0.0-16-hosted-crlf-lock, and its failing checks changed between the harness's own retry attempts: rollbackByteIdentical/rollbackOriginalBytes first, then serveEncodingIdentity/cleanRefusalWhileEncoded, with "transport failure; retrying" in between. The other 32 of 34 cases passed, and agent-two-versions passed on its 3rd attempt.
The same job passed on this PR's previous head 549f8c0 (job) and on main 61cfb9b. The only change since 549f8c0 is in .yarnrc --modules-folder parsing and reading. A vlt fixture has no .yarnrc or rush.json, so its crawl roots are byte-for-byte the same as before.

No fix to port exists, since nothing in the diff reaches this path. I'll re-run the failed job once when the workflow run finishes. If it fails again I'll treat it as real and root-cause it.

Generated by Claude Code

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026

Copy link

Copy Markdown

Collaborator Author

[agent] native (ubuntu-latest, 1.3.10) (Bun patch compatibility) also failed on 113ae51, with the same cause as the vlt job above. Between 02:28 and 02:35 UTC the harness got repeated Network error: error sending request for url (https://patches-api.socket.dev/patch/batch): … Connection reset by peer (and on /patch/view/…) from the production patch API. The cells that ran out of retries (direct hosted, custom-registry vendored) failed on those network errors, and 50 of 53 passed. Nothing in this diff touches the API client, and Bun fixtures have no .yarnrc or rush.json, so crawl roots are unchanged for them. I'll re-run the failed job once when the run finishes. The vlt job's re-run is already queued.

Generated by Claude Code

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

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

Oct 2, 2026

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026

Copy link

Copy Markdown

Collaborator Author

[agent] Ready for review at 113ae51.

CI: 383/383 non-skipped check runs green (6 skipped). The vlt 0.0.0-16 and Bun 1.3.10 native jobs each failed once during the 02:28–02:35 UTC patch-API Connection reset by peer outage. Both passed on their one re-run.
Bugbot: 2 findings on 549f8c0, both real and fixed in 113ae51 with regression tests (escaping/absolute --modules-folder values, FIFO .yarnrc blocking the crawl). It found no new issues on 113ae51. All threads are resolved.
Mergeable, 0 commits behind main.
For reviewers: .yarnrc --modules-folder parsing and the gate that only honors relative in-project subpaths (configured_install_roots / merge_configured_install_roots in npm_crawler.rs).

Generated by Claude Code

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026

Copy link

Copy Markdown

Collaborator Author

Reviewed 113ae515a7471f43924af6cb83fa277c15d4e3b6. Recommendation: fix before merging.

The default modules-folder and Rush paths are covered, but two valid Yarn Classic configurations still resolve to the wrong install root.

[P2] Resolve inherited modules-folder relative to the defining .yarnrc — crates/socket-patch-core/src/crawlers/npm_crawler.rs:52. yarnrc_modules_folder discards the defining config directory, then this join resolves the value against the project cwd. Yarn 1.22.22 resolves path options against the .yarnrc file itself. For /repo/.yarnrc containing --modules-folder project/deps and a scan of /repo/project, Yarn installs into /repo/project/deps, but this code looks for /repo/project/project/deps. Even though the actual tree is inside the project, apply misses it and VEX can fall back to lockfile-only evidence over an uninspected install. Retain the config file directory when parsing, resolve relative to it, and then enforce the project-subpath restriction on the resolved result.
[P2] Preserve install-specific modules-folder precedence — crates/socket-patch-core/src/crawlers/npm_crawler.rs:141. The parser combines --modules-folder and --install.modules-folder into one last-line-wins value. With --install.modules-folder specific followed by --modules-folder general, Yarn installs into specific/, while this crawler selects general/ (or no root if it does not exist). Yarn appends command-specific arguments after general arguments, independent of their order in the rc file; the same issue applies when these keys come from different ancestor configs. This leaves a valid configured tree unpatched/unverified. Merge each key through the rc hierarchy separately, then prefer the effective install-specific value, and cover both line orders.

Validation: cargo test -p socket-patch-core --lib npm_crawler: 71 passed. cargo test -p socket-patch-core --test review_520 (temporary reviewer regressions): Both regressions fail: expected install roots are present, but the crawler returns []. Yarn 1.22.22 install --ignore-scripts --offline --non-interactive fixtures: Confirmed directory-relative inherited configuration and install-specific flag precedence. Full workspace and external integration matrix not rerun.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.


          Resolve yarn modules-folder the way yarn 1.x does

ec95949

Two valid yarn classic setups still pointed the crawler at the wrong
install root, leaving the real tree unpatched and letting vex fall back
to lockfile-only evidence:

- A modules folder set in an ancestor .yarnrc is relative to that
  file's directory, not the project. /repo/.yarnrc with
  "--modules-folder project/deps" installs /repo/project into
  /repo/project/deps; the crawler looked in project/project/deps.
- --install.modules-folder wins over --modules-folder regardless of
  line order or which .yarnrc defines each, because yarn merges each key
  through the rc files separately and applies install-scoped args last.
  The crawler took whichever line came last.

The resolved folder must still lie strictly inside the project.
Verified against yarn 1.22.22 installs for all three layouts.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_012pqLNDF3U1KUabGfPsxoZ8

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026

Copy link

Copy Markdown

Collaborator Author

[agent] Thanks, both findings are confirmed and fixed in ec95949. I checked yarn 1.22.22's own lib/cli.js: loadRcFile resolves PATH_KEYS (including modules-folder) against dirname(.yarnrc); getRcConfigForCwd merges keys through the rc files one key at a time; and getRcArgs puts the install-scoped args after the * args.

Inherited modules-folder is relative to its .yarnrc: yarnrc_modules_folder now remembers which directory defined each key. resolve_modules_folder resolves the value from that directory, then keeps it only if the result is strictly inside the project; absolute or escaping values are still refused.
--install.modules-folder precedence: the parser now stores general and install separately (YarnrcModulesFolder). Each key comes from the nearest .yarnrc that sets it, and install wins if either file sets it.

Tests:

test_inherited_yarnrc_modules_folder_resolves_against_its_dir: /repo/.yarnrc project/deps gives project/deps, and a sibling deps is refused.
test_install_scoped_modules_folder_takes_precedence: both line orders, plus the two cross-file splits.
test_resolve_modules_folder: the cases for a value inherited from an ancestor.
I also corrected an existing case in test_yarnrc_modules_folder_is_a_crawl_root that had encoded the old cwd-relative behavior.

Both new crawl tests fail on 113ae51 ([] and [general]) and pass now. Real yarn install --offline 1.22.22 agrees in all three layouts (installs land in project/deps, project/specific, project/specific). Locally, npm_crawler is 71/71, clippy -D warnings is clean, and e2e_vex_redirect 27/27, e2e_vex_vendor 23/23 and in_process_alternate_installers 21/21 pass.

Generated by Claude Code

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026

Copy link

Copy Markdown

Collaborator Author

BugBot review

Generated by Claude Code

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

cursor Bot reviewed

Oct 2, 2026

View reviewed changes

cursor Bot left a comment

Copy link

Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

^{Bugbot Autofix is ON. A cloud agent has been kicked off to fix the reported issue.}

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

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

crates/socket-patch-core/src/crawlers/npm_crawler.rs Resolved

Uh oh!

There was an error while loading. Please reload this page.

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026

Copy link

Copy Markdown

Collaborator Author

Follow-up reviewed ec95949973c26c6c218d075fcbab6dcde7e65c2d. Both earlier findings are fixed; ready to merge from this review’s perspective.

Inherited modules-folder values now resolve against the .yarnrc directory, and install-specific options take precedence regardless of line order or which ancestor config defines them. The project-subpath restriction is preserved. No remaining actionable issue found in the update.

Validation on this head: both original reviewer regression tests pass (2 passed), and cargo test --locked -p socket-patch-core --lib npm_crawler passes (73 tests). Full workspace/platform matrix not rerun.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Tanmay Singla (Tanmay182003) approved these changes

Oct 2, 2026

View reviewed changes

Mikola Lysenko (mikolalysenko) merged commit 1ebdb11 into main

Oct 2, 2026

389 checks passed

Uh oh!

There was an error while loading. Please reload this page.

Mikola Lysenko (mikolalysenko) deleted the agent/fix-npm-crawler-configured-roots branch

October 2, 2026 16:15

Mikola Lysenko (mikolalysenko) mentioned this pull request

Oct 3, 2026

Agent mode ignores pnpm's modulesDir: on pnpm 10.12+ every installed package is "not installed", and apply exits 0 leaving it unpatched #661

Closed

Reviewers

cursor[bot]

cursor[bot] left review comments

Tanmay Singla (Tanmay182003)

Tanmay182003 approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

Ready for review

Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

npm crawler never looks in Rush's common/temp store: hosted vex attests a transitive dep not_affected while its installed copy is unpatched, and agent apply reports it package_not_installed Agent mode ignores yarn classic's --modules-folder: packages installed there are reported "not installed" and scan --mode agent exits 0 leaving them unpatched

Uh oh!

There was an error while loading. Please reload this page.

3 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

Conversation

Mikola Lysenko (mikolalysenko) commented Oct 2, 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

Change

Tests (red without the fix → green with it)

Review and CI

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 2, 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!

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 2, 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 2, 2026

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

cursor Bot left a comment •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

cursor Bot left a comment •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.