Skip to content

Fix npm crawler missing Bun, Deno and Yarn 4 stores (#366, #373, #405, #495) - #496

Merged
Mikola Lysenko (mikolalysenko) merged 10 commits into
mainfrom
agent/fix-npm-crawler-isolated-stores
Oct 2, 2026
Merged

Mikola Lysenko (mikolalysenko) merged 10 commits into
mainfrom
agent/fix-npm-crawler-isolated-stores

Conversation

@mikolalysenko

@mikolalysenko Mikola Lysenko (mikolalysenko) commented Oct 1, 2026 •

Copy link
Copy Markdown
Collaborator

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

Fixes #366
Fixes #373
Fixes #405
Fixes #495

Root cause

The npm crawler (crates/socket-patch-core/src/crawlers/npm_crawler.rs) knows isolated dependency stores only by hard-coded names and shapes: .pnpm, .vlt, the pnpm <=3 legacy .registry.*, a relocated virtualStoreDir, and npm's linked .store with a real node_modules/<name> dir. Under isolated linkers these stores are the only physical home of transitive dependencies.

#405 is the hosted/vendored VEX symptom of the same thing: the installed-copy lookup can't see .bun, so VEX takes the "nothing installed" branch.

Fix

  • PNPM_SHAPED_STORES (.pnpm, .bun, .deno) is now the one table all three walks consult, with a per-layout entry-name decoder. Bun names carrying a + tail are left undecoded, which keeps them probeable, so a peer hash is never mistaken for build metadata.
  • store_entry_own_package_sync accepts a store-entry link only when it resolves to that entry's own real package/ dir (Yarn 4). The copy is recorded at package/. Every other link in an entry is still a dependency edge. This is used by the scan, the resolver and the peer-copy fan-out.
  • Through that same link, the resolver also enqueues <entry>/package/node_modules, where the package's bundled dependencies live and where Node loads them from. The scan already descended there, so apply and scan now agree.
  • list_npm_store_entries_sync treats an @… store child that has its own node_modules as an entry, not a scope dir. That is Yarn 4's @scope-leaf-npm-… naming; an npm scope dir can never contain node_modules.
  • docs/ecosystems.md now lists the new stores.

Tests (red on main, green here)

Issue Regression test
#366 in_process_alternate_installers::bun_isolated_linker_transitive_only_dep_apply_patches_store: real bun install with linker = "isolated", apply patches .bun/is-number@6.0.0, and is-odd loads the patched copy. Also the core unit test test_bun_isolated_store_transitive_packages_are_found (scan, resolver, peer +hash twin, hosted-URL entry name, scoped).
#373 in_process_alternate_installers::deno_node_modules_dir_transitive_only_dep_apply_patches_store (a Deno 2.x-shaped .deno tree), plus test_deno_node_modules_store_transitive_packages_are_found (scan, resolver, _peer twin, scoped, .deno.lock, hoist dir).
#405 e2e_vex_redirect::bun_hosted_ref_is_judged_by_the_bun_store_copy: hosted bun.lock pin with the copy only in .bun, under both the stale (left-pad@1.3.0) and fresh (mangled-URL) entry names. A pristine copy now gives not_applied, where main falsely attested it (exit 0).
#495 e2e_yarn4_pnpm_linker_build::yarn4_pnpm_linker_agent_apply_patches_transitive_store_copy: real yarn 4.12.0 with nodeLinker: pnpm, apply patches .store/is-number-npm-6.0.0-*/package, yarn node loads it from is-odd, and rollback restores it. Also test_yarn4_pnpm_linker_store_transitive_packages_are_found.
#496 review test_yarn4_pnpm_linker_bundled_copy_inside_store_package_is_resolved and real yarn 4.12.0 yarn4_pnpm_linker_agent_apply_patches_bundled_copy_inside_store_package: a package bundling is-number@7.0.0 keeps that copy at .store/parent-…/package/node_modules/is-number, which Node loads. Apply patches it alongside the regular store copy, and rollback restores both.

On main each of these fails: package_not_installed (apply exit 1), or the VEX false attestation.

Local verification

  • cargo clippy --workspace --all-features -- -D warnings: clean. The workspace-wide cargo fmt --check reports the same pre-existing diffs as main (7f41839 reverted the unrelated reformat; CI doesn't run fmt).

  • cargo test -p socket-patch-core --lib npm_crawler: 67 passed, including the randomized oracle-equivalence tests.

  • cargo test --workspace --all-features --no-fail-fast: every suite passes except 14 tests in 5 targets that can't pass in this sandbox, all unrelated to the crawler:

    • 12 are chmod-based write-failure tests (covgap_commands_vendor, in_process_redirect, repair, and 4 core lib tests). The container runs as root, so the writes never fail.
    • 2 are mode_migration_npm berry takeover tests, which fail TLS against the sandbox proxy's CA.

    CI runs all of these as non-root, with normal network.

  • Real-toolchain e2e: bun 1.3.14 and yarn 4.12.0 legs pass, and each fails when run against main's crawler. The yarn 4 bundled-copy test (2d9eb4d) passes with SOCKET_PATCH_YARN_E2E_REQUIRED=1 and fails with the fix disabled.

  • CI: green on 3f8e0c8 (382/382 non-skipped, Windows included). 3f8e0c8 only fixes the bundled-copy e2e test's Windows path comparison. Bugbot clean on 2d9eb4d.

No wrapper changes are needed: npm/, pypi/ and gem/ only dispatch to the Rust binary.

🤖 Generated with Claude Code

https://claude.ai/code/session_01T2DjA5mBwq29D5rrFa5Fvd

Assisted-by: Claude Code:claude-opus-5-5
With Bun's isolated linker, Deno's isolated nodeModulesDir or Yarn 4's
pnpm linker, a transitive dependency lives only in the package
manager's store, which the crawler never looked in. Agent-mode apply
and scan reported those packages as not installed and left them
unpatched with exit 0, and VEX treated a hosted or vendored Bun pin as
having no installed copy, so it attested without checking the bytes.

The crawler now walks node_modules/.bun and node_modules/.deno like
pnpm's store, in scan, apply's resolver and the peer-copy fan-out. In
Yarn 4's node_modules/.store it finds each package at the entry's
package/ dir, which the entry's own node_modules link points to.

Assisted-by: Claude Code:claude-opus-5-5
Installs is-odd with the real yarn 4 pnpm linker, so is-number lives
only in node_modules/.store, then checks agent-mode apply patches it,
is-odd loads the patched copy, and rollback restores it.

Assisted-by: Claude Code:claude-opus-5-5
With bun.lock pinning the hosted tarball and the package installed only
in node_modules/.bun, a pristine store copy must be reported
not_applied, and a patched one attested. Before the crawler fix VEX saw
no installed copy and attested the pristine one.

Assisted-by: Claude Code:claude-opus-5-5
A real bun install with linker = "isolated" puts is-number only in
node_modules/.bun; apply must patch it and is-odd must load the
patched copy. A Deno-shaped node_modules/.deno tree gets the same
check without needing deno installed.

Assisted-by: Claude Code:claude-opus-5-5
Assisted-by: Claude Code:claude-opus-5-5
@mikolalysenko
Mikola Lysenko (mikolalysenko) marked this pull request as ready for review October 1, 2026 20:19
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

BugBot review


Generated by Claude Code

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

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.

b5985fc ran cargo fmt over the whole workspace, so 124 files outside
the crawler fix changed formatting only. That buried the real change
for reviewers and invites conflicts with every other open PR. Restore
those files to their main versions; npm_crawler.rs keeps its fix.

Co-Authored-By: Claude <noreply@anthropic.com>
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

bugbot run


Generated by Claude Code

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

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.

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

Copy link
Copy Markdown
Collaborator Author

Burn-down agent: ready for review at 7f41839 (7f41839d40ec197c4beeca245f497dc3d6d32348).

  • Change this run: b5985fc had run cargo fmt over the whole workspace, so 124 unrelated files changed formatting only. I checked that each one matched a cargo fmt of its parent exactly, then reverted all 124 to main. The diff is now 5 files: npm_crawler.rs, three new tests, and docs/ecosystems.md.
  • CI: 383/383 non-skipped check runs green (6 skipped).
  • Bugbot: reviewed 7f41839, no findings. No open review threads.
  • For the reviewer: the logic is all in crates/socket-patch-core/src/crawlers/npm_crawler.rs. It walks the .bun/.deno stores the same way as pnpm's store, and walks each entry's package/ dir under Yarn 4's .store.

Generated by Claude Code

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Reviewed 7f41839d40ec197c4beeca245f497dc3d6d32348. Recommendation: fix before merging.

The new Yarn 4 store support can report a successful apply while the package actually loaded at runtime remains unpatched.

  • [P1] Traverse the Yarn store package directory when resolving nested copies — [crates/socket-patch-core/src/crawlers/npm_crawler.rs:1192]( ). store_entry_own_package_sync now accepts Yarn 4’s self-link, but collect_nested_node_modules still ignores that symlink and never descends into the corresponding physical <entry>/package/node_modules. The scan path does descend there via gather_own_packages, so scan and apply disagree. With a real Yarn 4.12.0 pnpm-linker install of a tarball bundling is-number@7.0.0, Yarn creates both .store/is-number-.../package and .store/parent-.../package/node_modules/is-number; Node loads the latter. The resolver returns only the first, and apply_package_patch reports success while the runtime copy stays unpatched. When a store self-link resolves to its own real package directory, enqueue that directory’s nested node_modules for resolution as well; cover a bundled duplicate and verify the copy loaded by Node after apply.

Validation: cargo test -p socket-patch-core --lib npm_crawler: 67 passed. Real Yarn 4.12.0 yarn install with nodeLinker: pnpm and a tarball bundling is-number@7.0.0: Produced regular and bundled physical copies; Node loads the bundled copy. cargo test -p socket-patch-core --test review_496 (temporary reviewer regressions): Both tests fail: resolver returns one of two copies; core apply reports success but node -p "require('parent')" still returns the bundled unpatched sentinel. Full workspace matrix not rerun.

A Yarn 4 pnpm-linker store entry keeps its package at <entry>/package and
links <entry>/node_modules/<name> to it. The scan descends into that
package's own node_modules through the link, but the resolver never
follows links, so it missed bundled dependencies there. Apply then
patched the regular store copy and reported success while Node kept
loading the unpatched bundled copy.

When a store entry links to its own package dir, the resolver now also
enqueues that dir's node_modules.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T2DjA5mBwq29D5rrFa5Fvd
@mikolalysenko

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026 •

Copy link
Copy Markdown
Collaborator Author

Fixed the P1 in 2d9eb4d. Thanks for the repro.

The resolver now does what the scan does. When a store entry's node_modules/<name> links to that entry's own real package/ dir, the resolver also enqueues <entry>/package/node_modules. Apply and VEX's installed-copy lookup therefore now find bundled copies there. An entry with a sibling package/ dir but no link to it is still not walked.

Tests:

  • test_yarn4_pnpm_linker_bundled_copy_inside_store_package_is_resolved: the resolver returns all three is-number@7.0.0 copies: regular, bundled under parent, and bundled under a scoped @acme/tool. With the new line disabled it returns only the regular one.
  • e2e_yarn4_pnpm_linker_build::yarn4_pnpm_linker_agent_apply_patches_bundled_copy_inside_store_package: real yarn 4.12.0, nodeLinker: pnpm, with a local tarball that bundles a byte-identical is-number@7.0.0. The test asserts that require('parent') loads .store/parent-…/package/node_modules/is-number. Apply then patches both copies, yarn node loads the patched bundled copy, and rollback restores both. With the new line disabled, the "bundled copy patched" assertion fails.
  • test_store_package_dir_without_own_link_is_not_walked: a guard that an entry with a package/ dir but no link to it is not walked. It passes with or without the fix, and fails only if the new walk is widened past the self-link.

cargo test -p socket-patch-core --lib npm_crawler: 67 passed. Both yarn 4 agent e2e tests pass with SOCKET_PATCH_YARN_E2E_REQUIRED=1. cargo clippy --workspace --all-features -D warnings is clean.


Generated by Claude Code

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

BugBot review


Generated by Claude Code

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

[agent] Independent check of 2d9eb4d: I had drafted the same fix in parallel. I ran my two extra regressions against 2d9eb4d and both pass, so I dropped my branch and pushed nothing:

  • a unit test where find_by_purls must return all three copies of is-number@7.0.0: the regular .store entry, a copy bundled under an unscoped parent, and one bundled under a scoped @acme/host (reached through its @scope/name self-link);
  • an in-process agent apply on a fabricated Yarn 4 layout. It checks both copies get patched and that node -e "require(\"parent\")" loads the patched bundled copy.

Both fail on 7f41839 and pass on 2d9eb4d. CI on 2d9eb4d is still running, and I have asked Bugbot to re-review.


Generated by Claude Code

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ 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 2d9eb4d. Configure here.

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Follow-up reviewed 2d9eb4dd12c4f5f46b338986477f5794af74af21. The earlier P1 is fixed; ready to merge from this review’s perspective.

The resolver now visits bundled dependencies inside a Yarn 4 store entry’s physical package/node_modules after verifying its own package link. The original missing-copy regression passes. The new real Yarn regression also confirms that CLI apply patches both the regular and bundled copies, Node loads the patched bundled copy, and rollback restores both. No remaining actionable issue found in the update.

Validation on this head: original reviewer resolver regression 1 passed; cargo test --locked -p socket-patch-core --lib npm_crawler 69 passed; required yarn4_pnpm_linker_agent_apply_patches_bundled_copy_inside_store_package end-to-end test 1 passed, no skip with Node 24.21.0. Full workspace/platform matrix not rerun.

Claude (claude) and others added 2 commits October 2, 2026 15:02
The bundled-copy e2e test matched Node's resolved path against a
forward-slash substring, which fails on Windows where Node prints
backslashes. Compare the canonical path of what `require('parent')`
resolves to with the bundled copy's instead.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T2DjA5mBwq29D5rrFa5Fvd
Keep the Bun, Deno, and Yarn 4 store regressions alongside main's Yarn modules-folder and Rush install-root regressions when resolving the appended-test conflicts. Production changes from both parents are preserved.

Signed-off-by: Mikola Lysenko <mikolalysenko@gmail.com>
@mikolalysenko

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026 •

Copy link
Copy Markdown
Collaborator Author

Fixed the merge conflicts and re-reviewed 0cb726ee6e17671ed0922eed25806e7248e115ea (merges main at 73b17db5). Ready to merge from code review; CI on this updated head is now green.

All three conflicts were overlapping appended tests. Both regression sets are preserved: Bun/Deno/Yarn 4 isolated stores, plus Yarn’s configured modules folder and Rush’s shared install root. No production implementation was discarded, and no further actionable defect was found.

Validation: 78 crawler tests, 3 hosted VEX tests, 3 apply fixtures, and 2 real Yarn 4 pnpm-linker tests pass. The Yarn tests verify runtime-loaded bytes and rollback, including bundled copies; they ran in required mode without skips. Tests ran on macOS with Node 24.21.0; real Bun installation and the full platform matrix were not rerun locally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment