Fix path-scoped scan missing member alias links (#1266)

[Mikola Lysenko (mikolalysenko)]

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

Fixes #1266

What users saw

In a workspace using pnpm's default isolated linker, or npm with
install-strategy=linked, a member that depends on an alias
("lp": "npm:left-pad@1.3.0") gets packages/a/node_modules/lp as a
symlink into the root store:

• scan --mode agent packages/a never selected left-pad, so the copy
  member a loads stayed unpatched. It reported success and exited 0.
• rollback packages/a left the patched bytes in place (npm: success;
  pnpm: exit 1 path_glob_no_match).

The member's plain-name links (packages/a/node_modules/ms) worked.

Root cause

Path-scoped scan and rollback match patterns against the paths
find_all_packages_for_rollback_reusing returns, which are the npm
crawler's copies. The crawler records a plain-name link at the member
path, but never takes an alias link as a copy. That rule is
deliberate (#856): apply and VEX must see each physical copy once, and
the vex_consumed tests pin it. Path scoping asks a different question:
which install paths reach the package.

Fix

A new `find_package_paths_for_path_scope` (used only by `scan`'s and
`rollback`'s path targets) returns the physical copies plus every
package link directly in a `node_modules` root (scoped ones one level
down) whose target's own `package.json` name@version is one of the
requested npm purls. Links are matched by identity in one pass over the
root links, so a member alias pointing at a store peer variant the crawl
skipped still counts. Apply, VEX and rollback's restore list are
unchanged. `CLI_CONTRACT.md` ("Path-scoped scans") now describes this.

A first attempt changed the crawler instead. It broke the #856
`vex_consumed` invariants, so I dropped it. A second version
(84b3690) expanded store peer variants once per purl. Review measured
it as quadratic on big pnpm stores, and 4288a72 replaces it:

`scan packages/a --mode agent --dry-run`, debug build	main	84b3690	4288a72
pnpm store, 1,000 entries	0.10 s (review)	4.7 s	0.10 s
pnpm store, 3,000 entries	0.24 s (review)	34.6 s	0.24 s

Tests (red on main, green here)

Issue shape	Test
#1266 pnpm isolated, scoped scan	scan::scan_paths_e2e::paths_scope_selects_pnpm_member_alias_link
#1266 npm linked (npm 11+ and npm 10 store-entry spellings), scoped scan	scan::scan_paths_e2e::paths_scope_selects_npm_linked_member_alias_link
#1266 member alias resolving to a different pnpm peer variant	scan::scan_paths_e2e::paths_scope_selects_member_alias_link_to_a_peer_variant
#1266 rollback packages/a (pnpm .pnpm and npm .store)	rollback::rollback_duality_invariants::path_scoped_rollback_selects_member_alias_link

Without the fix, the scan tests fail with a batch body of only
{"components":[{"purl":"pkg:npm/ms@2.1.2"}]}, and the rollback test
fails on the exit code. The peer-variant test also fails if the
variant expansion is removed.

Local runs:

• cargo fmt --all -- --check: clean
• cargo clippy --workspace --all-features -- -D warnings: clean
• cargo test --workspace --all-features --no-fail-fast: everything
  passes except 13 tests that also can't pass in this sandbox on
  main. Most are write-failure tests that need chmod-denied writes,
  and the sandbox runs as root. One bun lockb test needs network access
  to patches-api.socket.dev. None of them touch path scoping, and the
  core crate is unchanged.

/code-review high found 6 issues and all of them are fixed in
84b3690: the peer-variant gap, moving the link pass out of rollback's
restore resolver, skipping the root clone when there are no npm purls,
no panic on a join error, Ecosystem::from_purl in place of a prefix
test, and folding a duplicate test scan into the loop.

🤖 Generated with Claude Code

https://claude.ai/code/session_011KEL5sGU2JuwHvDCpJtMwi

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

BugBot review

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

BugBot review

---

Generated by Claude Code

[cursor]

Stale Bugbot comment from a previous run.

[Mikola Lysenko (mikolalysenko)]

Labeled Ready for review by the burn-down agent.

• Head: 84b3690169 (mergeable against main, no conflicts; CHANGELOG.md untouched)
• CI: 36/36 green + 50 path-skipped; no main-wide failures
• Bugbot: reviewed 84b3690, no new issues; no open review threads
• Reviewer focus: how path-scoped scan/rollback <member> now pick up workspace member alias links (ecosystem_dispatch.rs, scan/mod.rs) and the new scan_paths_e2e.rs / rollback duality cases.
• Slack announcement not sent: the Slack send tool isn't available to this run.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Final review: blocking perf regression on path-scoped scan

The fix is correct: the 4 new tests pass on this head and fail on the merge-base. However, it makes path-scoped scan on a pnpm store scale as O(crawled purls × store entries), so I can't brief it for approval yet.

Cause

• In scan, scan/mod.rs:2183 calls the new find_package_paths_for_path_scope for every crawled purl that isn't directly in scope. In a monorepo that is almost the whole install.
• For each npm purl, ecosystem_dispatch.rs:667 runs with_store_peer_variant_copies. Each call builds a fresh scanned set, re-lists the whole .pnpm/.store/.vlt store, stats each entry's node_modules and probes the ancestors.
• Before this PR, the scoped path did a single dispatch_find.

Measured with debug builds on a synthetic pnpm root: N store entries, root links to each, and member packages/a with one link. Command: scan packages/a --mode agent --dry-run --json.

Store entries	merge-base	this head
1,000	0.10 s	6.9 s
3,000	0.24 s	53 s

Unscoped scan on this head stays at 0.16 s for N=3,000. Release builds are faster but still quadratic, so a store of about 5,000 entries would take minutes.

Suggested fix (any one)

• Enumerate the node_modules root links once, canonicalize each, read the target's package.json name@version, and match it against the unresolved purls. This avoids per-purl variant expansion.
• Or expand variants only for purls with a link target that matched no known copy.
• Or share one scanned set and one store listing across all purls.
• A large-store regression test or bench would keep this from coming back.

Non-blocking

• Root node_modules links now select too (e.g. node_modules/left-pad). That fits the semantics, but the CLI_CONTRACT.md clause only describes member aliases.
• The new tests are all #[cfg(unix)].
• find_all_packages_for_rollback_reusing may have no non-test caller left.

Question for a human: is the scoped-scan slowdown acceptable to ship, or should this wait for one of the fixes above? I recommend the fix. I've labelled this agent:needs-human and removed Ready for review; whoever picks it up can drop the label once it's fixed.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

[agent] Thanks, the regression was real. Fixed in 4288a72.

• Path scoping no longer expands peer variants per purl. It enumerates the node_modules root links once, canonicalizes each, reads the target's package.json name@version and matches it against the requested npm purls (your first suggestion). A link to a peer variant the crawl skipped still counts. The peer-variant regression test still passes.
• Same synthetic pnpm root as yours (N store entries, root links to each, packages/a with one alias link), debug build, scan packages/a --mode agent --dry-run --json:

Store entries	84b3690	4288a72
1,000	4.7 s	0.10 s
3,000	34.6 s	0.24 s

Both select the same single package. 4288a72 matches your merge-base numbers.

• Non-blocking: CLI_CONTRACT.md now describes any root link, matched by its target's identity, not only member aliases. The tests stay #[cfg(unix)] because they create symlinks; the other symlink path-scope test in this suite is unix-only for the same reason. find_all_packages_for_rollback_reusing keeps its test callers (the snapshot-equivalence tests), so I left it in place.
• I didn't add a timing-based large-store test because it would be flaky on CI runners. The bench above is easy to rerun if you want one as a follow-up.

I'm removing agent:needs-human as your note says to once this is fixed.

---

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 4288a72. Configure here.}

[Mikola Lysenko (mikolalysenko)]

Labeled Ready for review by the burn-down agent.

• Head: 4288a72bbd18bb6e59cc7eb620b2a37ad7dabbc9 (mergeable against main, no conflicts; no CHANGELOG.md change)
• CI: 35/35 green (19 skipped) on the head, no main-wide failures
• Bugbot: reviewed 4288a72, no new issues; no unresolved review threads
• Reviewer note: the earlier final-review perf regression on path-scoped scan was addressed in 4288a72 (no longer expands peer variants per purl).
  Slack announcement not sent this run (Slack send tool unavailable); the next run will retry.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Final review brief

What it does: Path-scoped scan (agent mode) and rollback decided scope from the crawler's list of physical copies, which on purpose leaves out npm alias links. So a member's node_modules/lp -> npm:left-pad@1.3.0 link into the pnpm or npm-linked store was never in that member's scope. The new find_package_paths_for_path_scope adds every link at the top of each node_modules root (and under @scope dirs) whose target's package.json name@version is a requested purl. Scan and rollback use it only to decide which purls are in scope.

Risk: low. It only adds match candidates for path-scoped runs. Writes still go through the crawled copies, so it can't touch anything an unscoped run wouldn't. Links are followed read-only (canonicalize plus a package.json read).

Look here

• ecosystem_dispatch.rs:680: which roots get searched (reuses the earlier crawl's roots, skipped when no npm purls are requested)
• ecosystem_dispatch.rs:733: identity, where links are followed
• ecosystem_dispatch.rs:752: link enumeration, including scoped and hidden entries
• rollback.rs:1594 and scan/mod.rs:2183: the two call sites

Verified

• Read the full diff.
• Test binaries pass: scan 128/128, rollback 58/58, and the ecosystem_dispatch unit tests 46/46.
• With src/ reverted to main, all 4 new tests fail, so they do test the fix.
• A throwaway probe with a self-loop link, a dangling link and a scoped link pointing outside the project: no hang and no error. Broken links are skipped.
• Clippy is clean on the changed crate's lib and bins.
• CHANGELOG.md is untouched and there's no debug code.
• CI is green on this head (ci-ok, clippy). Bugbot reviewed this head and left no open threads.
• I didn't re-measure the perf table, but the link pass is one walk per root and doesn't depend on how many purls are requested, which fits the claim.

Changes I made: none.

Open questions (non-blocking)

• CLI_CONTRACT.md:213 still says path matching uses "no filesystem access or symlink resolution". The glob match itself doesn't, but which purl a link belongs to is now decided by resolving the link. That clause could say so.
• All the new tests are #[cfg(unix)]. Windows junctions should be found, since is_symlink is true for them, but no test covers it.
• If a member links a workspace sibling whose name@version matches a registry package, that purl is selected. main already does this for plain-name links, so it isn't a regression.

Auto-merge is armed, so approving sends this straight to the merge queue.

---

Generated by Claude Code