Fix npm store copies missed by agent apply and vex (#601, #603)

[Mikola Lysenko (mikolalysenko)]

Final-head CI is complete: 452 successful checks, 6 skipped; no failures or pending checks. Bugbot passed, there are no unresolved review threads, and the PR is mergeable.

Fixes #601 and #603. In pnpm, vlt, Bun and Deno layouts, agent apply, rollback and vex must discover every relevant physical copy. Previously, finding a normal installation could hide a copy bundled inside another store entry; agent VEX also omitted peer variants that apply already patched. VEX could therefore attest a package while a consumed copy remained unpatched.

The resolver now visits the owning package's bundled node_modules even when its store entry is skipped by the pending-name filter. Agent VEX uses the shared npm store-variant expansion so an unpatched bundled or peer copy prevents attestation. CLI_CONTRACT.md and CHANGELOG.md describe the every-copy behavior.

Review corrections preserve that coverage while bounding traversal and repeated work:

• Each resolver pass tracks canonical node_modules directories separately for importer and store-entry mode. Cyclic bundled links terminate; root-first path choices, legitimate linked copies, and the two different link policies are retained.
• Aggregate peer expansion enumerates each canonical store once per layout, package name and version. Every input still contributes its lexical, canonical and relocated candidate stores, so a later alias can reveal another store. The cache exists only for that aggregate call; standalone apply/rollback discovery gets fresh state.
• Hosted VEX reuses the expanded installed set, expands new alias paths before canonical merging, and continues to expand identity-fallback paths when the installed entry is empty.

Validation on b92456b3:

• 228 focused tests passed: 80 npm core unit tests, 85 crawler integration tests, 10 hosted-copy unit tests, 5 multicopy apply/rollback tests, 18 agent VEX tests, 28 hosted VEX tests, and 2 independent reviewer controls. These cover the original bundled/peer-copy regressions as well as alias, fallback, scope and linked-tree behavior.
• Deterministic regressions fail before the correction and pass after it: an eight-copy transitive resolver result scans its store once, mixed identities and alias stores retain the complete copy set, and hosted verification does not expand already-expanded installed copies again.
• The linked-directory reproducer exceeded a two-second process bound on the original PR head and now resolves in about 1.44 ms. A bounded 128-copy expanded-input helper probe dropped from about 6.24 s to 224 ms. These are local component measurements, not whole-command benchmarks or portable timing guarantees.
• Targeted formatting and diff checks passed. Clippy passed for both libraries with unused_variables allowed for the existing macOS Python-crawler warning. The final committed files match the tested hashes; independent source/evidence review found no remaining issue, and the commit merges cleanly with main 045d7ec7.
• Full CI, compatibility workflows, benchmarks, and Bugbot completed successfully on the corrected commit b92456b3. A Windows Bun 1.2.23 patch-detail API timeout was retried once at the failed-job level; all 49 cases passed on the retry, including the expected workspace refusal.

#599, concerning orphaned Bun store entries, remains a separate reachability change.

---

Note

Medium Risk
Changes npm install discovery and VEX copy sets for security-sensitive apply/verify paths; bounded traversal and deduplication reduce performance and infinite-loop risk but behavior shifts when multiple physical copies exist.

Overview
Fixes #601 and #603 so agent apply, rollback, and vex treat every physical npm copy that can actually run—not only the “normal” importer-linked install.

The npm resolver now walks bundled node_modules inside skipped pnpm/vlt/Bun/Deno store entries when the same name@version was already found elsewhere, with cycle-safe traversal so bundled trees that link back to ancestors do not hang or drop legitimate copies.

with_store_peer_variant_copies centralizes peer/index/alias store expansion (with one scan per store identity per batch). Manifest copy lookup applies it for npm; hosted vex reuses already-expanded installed paths and only expands new alias paths, avoiding redundant store rescans.

Docs (CHANGELOG, CLI_CONTRACT) state that agent verification must hash every crawler copy, including peer variants and bundled store copies. Regression tests cover apply/rollback on bundled layouts, agent VEX omission when any store copy is pristine, and hosted-merge behavior.

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

[Mikola Lysenko (mikolalysenko)]

BugBot review

---

Generated by Claude Code

[cursor]

Stale Bugbot comment from a previous run.

[Mikola Lysenko (mikolalysenko)]

[burn-down agent] Labeled Ready for review at a513449c43d3f82f1f7da8fa0a92e15a6dafe7f4.

• CI: 452/458 check runs green on this head (6 skipped by path/matrix filters), 0 failing.
• Bugbot: reviewed this exact head, no findings; no unresolved review threads.
• Mergeable, up to date with main @ 045d7ec.
• Reviewer focus: npm_crawler.rs per-purl copy enumeration now including store copies (bundled/peer variants), and the matching vex every-copy rule in vex_consumed.rs / CLI_CONTRACT.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Review updated for b92456b37893a09fad16991058ba6aa0d5238bf5: Ready to merge as-is from this review. Final-head CI is complete: 452 successful checks, 6 skipped; no failures or pending checks. Bugbot passed, there are no unresolved review threads, and the PR is mergeable.

The correction bounds repeated npm store scans and bundled-directory cycles while preserving every-copy coverage:

• Hosted VEX reuses the expanded installed set and expands only new aliases, with identity fallback still covered. The shared helper enumerates each store/layout/name/version once per aggregate call, including transitive-only resolver results; aliases can still expose different stores.
• Resolver visits are deduplicated by canonical directory and importer/store mode per pass. Cycles terminate while legitimate linked copies and root-first paths remain discoverable.

All 228 focused tests passed, including multicopy apply/rollback, agent and hosted VEX, crawler oracles, and the new work-count/cycle regressions. The original cycle reproducer exceeded two seconds and now resolves in about 1.44 ms. The 128-copy expanded-input helper measurement improved from about 6.24 s to 224 ms; these are component timings, not whole-command benchmarks. Deterministic tests establish the store-scan bound.

The committed files match the tested source. Independent review found no remaining actionable issue; formatting, diff checks, and targeted clippy passed (with the existing macOS unused_variables allowance). The corrected commit merges cleanly with current main.

No remaining code finding from this review. The Ready label has been restored after all checks completed on the corrected commit.

CI note: the initial Windows Bun 1.2.23 job failed because its patch-detail API call timed out before the expected refusal. Artifact 11256446880, captures/1.2.23-workspace-vendored/cli-output.json, records all 1 patch-detail queries failed and operation timed out; all no-mutation checks passed. One targeted job retry passed all 49 cases with the same binary, the correct refusal, and unchanged project files.

[Mikola Lysenko (mikolalysenko)]

Confirmed on a513449: find_manifest_package_copies_reusing already applies with_store_peer_variant_copies to every npm purl, and hosted_consumed_copies (vex_consumed.rs:112) then runs it again over that expanded list. The second pass is only needed for the paths the hosted step adds itself: npm_alias_copies_reusing and npm_identity_fallback_reusing.

You've said you're preparing the correction on this branch, so I'm not pushing a competing one. The fix I'd propose expands only those additions and leaves the installed paths as they are:

for purl in shared.values().flatten() {
    let installed_paths = installed.get(purl).cloned().unwrap_or_default();
    let mut paths = all.remove(purl).unwrap_or_default();
    paths.extend(aliases.remove(purl).unwrap_or_default());
    if npm.contains(&purl) {
        // `installed` is already store-variant expanded (#603); expand only
        // what the alias walk and identity fallback added here.
        let added: Vec<PathBuf> = paths
            .into_iter()
            .filter(|p| !installed_paths.contains(p))
            .collect();
        let mut merged = installed_paths;
        for p in with_store_peer_variant_copies(added).await {
            if !merged.contains(&p) {
                merged.push(p);
            }
        }
        paths = merged;
    }
    // ...
}

This keeps every-copy verification for agent records, and aliased or fallback copies still get their variants. For a purl with no alias or fallback copies, the store scan count goes back to one. The existing e2e_vex store-copy cases and the hosted alias and fallback tests in vex_consumed.rs should cover it.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Cursor (@cursor) review

Both review findings are corrected in b92456b37893a09fad16991058ba6aa0d5238bf5: bounded bundled-directory traversal and store expansion reuse. Please review this updated head.

[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 b92456b. Configure here.}

[Mikola Lysenko (mikolalysenko)]

Update (23:20 UTC): the Bun patch compatibility workflow on b92456b passed on a re-run (attempt 2), including native (windows-latest, 1.2.23). Every workflow on this head is now green and Bugbot is clean. That takes back my "probably caused by this PR" below: the failure did not reproduce on the same commit.

It is still unexplained. If workspace vendored fails on Windows again, the refusal codes in that run's bun-results-windows-* artifact (captures/**/workspace*vendored*/cli-output.json) are what to look at first.

Original report

CI on b92456b: Bun patch compatibility / native (windows-latest, 1.2.23) failed on attempt 1 (job). One scenario out of 49 failed: 1.2.23 workspace vendored FAIL ['refusalCodesExact']. The same job passed 49/49 on a513449, and Ubuntu with Bun 1.2.23 passed on this head. I suspected the Windows canonical-path handling in the new visited set. I couldn't download the artifact from my sandbox to check the actual codes.

---

Generated by Claude Code