GeezerVison

BugBot review

Generated by Claude Code

bugbot run

Generated by Claude Code

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

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](

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

  Line 1192 in 7f41839

  })

  ). 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 [email protected], 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 [email protected]: 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.

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 [email protected] 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 [email protected]. 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

BugBot review

Generated by Claude Code

[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 [email protected]: 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

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.

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.