From 6f2d2dc2e2692ad29c3d69a0429fdd94863e05ed Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 07:24:28 +0000 Subject: [PATCH 1/4] Start fix for #1266 Assisted-by: Claude Code:claude-opus-5-5 From 88c91ffafaa7c6f3fa8b32e16d57388b3f41755c Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 07:56:24 +0000 Subject: [PATCH 2/4] Scope member alias links in path-scoped runs Under pnpm's default isolated linker and npm's linked install strategy, a workspace member's alias dependency ("lp": "npm:left-pad@1.3.0") is a symlink from packages/a/node_modules/lp into the root store. `scan --mode agent packages/a` never selected it and `rollback packages/a` never restored it, both reporting success, while the member's plain-name links were handled. Path scoping now also counts every package link in a node_modules root that resolves to one of a package's installed copies. The crawler itself still records only physical copies, so apply and VEX see each copy once, as before. Fixes #1266 Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-cli/CLI_CONTRACT.md | 2 +- .../src/ecosystem_dispatch.rs | 103 ++++++++++++++- .../rollback/rollback_duality_invariants.rs | 94 ++++++++++++++ .../tests/scan/scan_paths_e2e.rs | 121 ++++++++++++++++++ 4 files changed, 315 insertions(+), 5 deletions(-) diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index 6506b62ad..b51148203 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -210,7 +210,7 @@ For a **9.0 root lock**, the CLI ensures `pnpm-workspace.yaml` carries `trustLoc **Path-scoped scans (`scan [PATHS]...`, v5.0)**: what a PATH means depends on the mode. * **Hosted and vendored mode (bare `scan` included) — project directories** (`run_project_dirs`). Each PATH is a directory, or a glob (`*?[`) matching directories, relative to `--cwd`; the set is sorted and deduplicated, and each directory is scanned on its own exactly as if it were `--cwd` (its own lockfiles, ledgers and `.socket/`). With more than one directory, each run is headed `== ==` on stdout (unless `--silent`), and the exit code is the worst of the runs. Usage errors (exit 2, stderr only, before any scan): a PATH that is not a directory (`` `X` is not a directory``), a glob matching no directory (`` `X` matches no directory``), an invalid glob, and `--json` with more than one directory (`--json takes one project directory (N given); run one scan per directory`), so stdout stays one document. Likewise `--vex` with more than one directory (`--vex takes one project directory (N given); run one scan per directory`): the one output path would be overwritten by each run. -* **Agent mode (and a mode-less `--prune`/`--global` report) — installed-path globs** scoping DISCOVERY at the **purl level**: a package is in scope iff ANY of its crawled installed copies sits under a matching path (every copy, enumerated the way `rollback`'s path targets enumerate them: a pnpm workspace member's link into the root `.pnpm` store and a nested member copy count, not just the one copy the crawl records per purl), and a selected package is then handled with ALL its copies (scoping selects which packages are considered, never which copies). Glob semantics (shared with `rollback`'s path targets, `src/path_scope.rs`): Unix-shell globs with `require_literal_separator` — `*`/`?` never cross a `/`, `**` spans directories; a pattern matching any **ancestor** directory of the copy path also matches, so a bare `scan packages/foo` scopes the whole subtree without `/**`; relative patterns match against the copy path relativized to `--cwd`, absolute patterns against the absolute path (the ONLY way to reach paths outside the project tree, e.g. `--global` stores — a relative pattern never matches outside `--cwd`); leading `./` and trailing `/` are normalized away, matching is purely textual (no filesystem access or symlink resolution), case-sensitive except on Windows (whose filesystems are not); an unparseable or empty pattern is a usage error (exit 2). **The prune universe is never narrowed**: the path filter is applied strictly AFTER the `scanned_purls` capture (and after `--ecosystems`), so `scan PATHS --prune` prunes exactly what an unscoped `scan --prune` would — a scoped scan can never treat an out-of-scope package as uninstalled (the same fail-safe as the `--ecosystems` filter). Lockfile-only and vendor-ledger supplement records have no installed path and are EXCLUDED from a path-scoped scan, surfaced as one run-level `path_scope_excluded_supplements` warning carrying the count. A scope matching nothing is a normal empty scan — exit 0, zero packages, **no GC** (the zero-package early return fires before any GC). `PATHS` combine with `--mode agent`/`--sync`/`--prune`/`--global`. Every scan JSON envelope past the argument checks (success, zero-package, all-batches-failed) carries a `paths` key echoing the patterns verbatim (empty array when unscoped; a hosted/vendored per-directory run is unscoped, so it is `[]`). One-sentence duality rule: **a target that selects nothing is an error on `rollback` (exit 1) and an empty scan on an agent-mode `scan` (exit 0)**. +* **Agent mode (and a mode-less `--prune`/`--global` report) — installed-path globs** scoping DISCOVERY at the **purl level**: a package is in scope iff ANY of its crawled installed copies sits under a matching path (every copy, enumerated the way `rollback`'s path targets enumerate them: a pnpm workspace member's link into the root `.pnpm` store (or npm's linked `.store`), including an alias link such as `packages/a/node_modules/lp` for `"lp": "npm:left-pad@1.3.0"`, and a nested member copy count, not just the one copy the crawl records per purl), and a selected package is then handled with ALL its copies (scoping selects which packages are considered, never which copies). Glob semantics (shared with `rollback`'s path targets, `src/path_scope.rs`): Unix-shell globs with `require_literal_separator` — `*`/`?` never cross a `/`, `**` spans directories; a pattern matching any **ancestor** directory of the copy path also matches, so a bare `scan packages/foo` scopes the whole subtree without `/**`; relative patterns match against the copy path relativized to `--cwd`, absolute patterns against the absolute path (the ONLY way to reach paths outside the project tree, e.g. `--global` stores — a relative pattern never matches outside `--cwd`); leading `./` and trailing `/` are normalized away, matching is purely textual (no filesystem access or symlink resolution), case-sensitive except on Windows (whose filesystems are not); an unparseable or empty pattern is a usage error (exit 2). **The prune universe is never narrowed**: the path filter is applied strictly AFTER the `scanned_purls` capture (and after `--ecosystems`), so `scan PATHS --prune` prunes exactly what an unscoped `scan --prune` would — a scoped scan can never treat an out-of-scope package as uninstalled (the same fail-safe as the `--ecosystems` filter). Lockfile-only and vendor-ledger supplement records have no installed path and are EXCLUDED from a path-scoped scan, surfaced as one run-level `path_scope_excluded_supplements` warning carrying the count. A scope matching nothing is a normal empty scan — exit 0, zero packages, **no GC** (the zero-package early return fires before any GC). `PATHS` combine with `--mode agent`/`--sync`/`--prune`/`--global`. Every scan JSON envelope past the argument checks (success, zero-package, all-batches-failed) carries a `paths` key echoing the patterns verbatim (empty array when unscoped; a hosted/vendored per-directory run is unscoped, so it is `[]`). One-sentence duality rule: **a target that selects nothing is an error on `rollback` (exit 1) and an empty scan on an agent-mode `scan` (exit 0)**. `scan --mode vendored` swaps the in-place apply for the vendor pipeline: discover → download the selected patch records **into memory** (no manifest write) → vendor every selected dependency via the same engine as the `vendor` command (under the same lock). Vendored mode is **manifest-free (v5.0)**: `.socket/manifest.json` is never written or read by a vendored run; each ledger entry carries `detached: true` plus an embedded copy of the patch record (`record`) as its verification source, and the run's footprint is `.socket/vendor/**` only. The vendor step's scope is what discovery selected — the former "whole manifest is vendored" re-vendor on an empty discovery is retired (`repair` verifies and redownloads committed vendored state; `scan --prune` reconciles ledger entries whose dependency left the lockfile). The vendor-ledger discovery supplement (the fresh-clone rule: a ledger entry with no installed copy stays discoverable because its committed artifact IS the dependency) holds only while the lockfile still resolves through that artifact: an entry the lockfile in-use probe (the one `--prune` reverts by) proves unwired, because the dependency was upgraded or removed, is NOT discovered and so is never re-vendored. A run without a non-hosted `--prune` reports it through the run-level `vendor_ledger_entry_unwired` warning; a `--prune` run reverts it in its GC and exits 0. That GC runs even when the crawl found no packages, as its vendored half alone (the manifest prune stays skipped there). A package the ledger holds at an older patch uuid is still **re-vendored automatically** when discovery selects the newer patch (its old uuid dir is removed — `vendor_stale_artifact_removed`) — unless the patch service has no prebuilt artifact for the newer patch yet (or at all): an npm-family package then keeps the older patch and is a `skipped` `vendor_prebuilt_pending` / `vendor_prebuilt_unavailable` event, not a failure (#954); same-uuid re-runs reuse the embedded record, skip the patch-view fetch, and are `already_vendored` skips. **Legacy manifest-mode entries**: when a vendored run vendors a purl that also has a `.socket/manifest.json` record (a project vendored by a pre-5.0 binary, or by standalone `vendor` from an agent-mode manifest), that manifest record is dropped in the same run — the ledger becomes the owner (migration write); an emptied manifest is left as `{"patches": {}}`, never deleted. The migration is reported through the run-level `warnings[]` (stderr in human mode), never as a run error: `vendor_manifest_record_migrated` (`N manifest records moved to the vendor ledger (vendored mode is manifest-free): `) or `vendor_manifest_migration_failed` (the manifest or the ledger could not be read or rewritten; the legacy records were left in place) — so a corrupt `.socket/manifest.json` no longer fails a vendored run (standalone `vendor`, the one manifest-driven writer, still fails closed on it). With `--prune`, GC runs **after** the vendor step (the step never reads the manifest, and running the sweep last lets it reclaim what the run itself orphaned — a migrated legacy record's blobs, a superseded uuid dir). Under `--json` (v5.0, MAJOR) the download phase and the vendor engine record into the ONE scan envelope (no nested `download` / `vendor` objects): a fetched patch is a `downloaded` event (the patch metadata in `details`, plus `details.oldUuid` when the ledger holds the purl at another uuid — the re-vendor the vendor step then performs — whose human `[fetch]` line reads ` (replacing )`), a refused or failed one a `failed` event with its code, a patch the ledger already holds at the selected uuid no download event at all (reused, no fetch); then the vendor engine's events (`applied`, `rebuilt`, `skipped`, `failed`, `removed`, its uncounted advisories) follow with their warnings and sidecars. Every one of these events carries `details.mode: "vendored"`. Patch blobs are held in memory (see "Patch sources stay in memory" under the vendor contract). `--dry-run` previews per patch: `verified` (to vendor; `oldUuid` on a re-vendor), `skipped` / `already_vendored` (in sync), or `skipped` with the refusal code for an npm purl the wet run's Bun preflight (see the `get --mode vendored` bullet below) would refuse, or whose hosted pnpm pin the takeover would be refused on (see "Takeover reconciliation") — without network downloads or disk writes; the preview never flips status or exit (the human path — `scan` and `get` alike, through one shared printer — prints `[would-refuse] (): ` lines behind the `--silent` gate). Interactive mode prompts "Download and vendor N patches?" (singular for one). diff --git a/crates/socket-patch-cli/src/ecosystem_dispatch.rs b/crates/socket-patch-cli/src/ecosystem_dispatch.rs index 7279a14b5..edba73152 100644 --- a/crates/socket-patch-cli/src/ecosystem_dispatch.rs +++ b/crates/socket-patch-cli/src/ecosystem_dispatch.rs @@ -4,7 +4,7 @@ use socket_patch_core::crawlers::{ use socket_patch_core::utils::purl::strip_purl_qualifiers; use socket_patch_core::utils::purl_key::PurlKey; use std::collections::{HashMap, HashSet}; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use crate::args::GlobalArgs; @@ -637,16 +637,111 @@ pub async fn find_all_packages_for_rollback( /// from `prior` as [`find_packages_for_rollback_reusing`] does. Used by /// `scan`'s path scoping, which must see the same copies `rollback`'s path /// targets do. +/// +/// An npm package's copies here also include each link in a `node_modules` +/// root that resolves to one of them ([`add_npm_links_to_copies`]): path +/// scoping asks which install paths reach a package, and pnpm's isolated +/// linker or npm's `install-strategy=linked` reach a workspace member's +/// ALIAS dependency only through such a link (#1266). pub async fn find_all_packages_for_rollback_reusing( partitioned: &HashMap>, options: &CrawlerOptions, silent: bool, prior: Option<&NpmCrawlSnapshot>, ) -> HashMap> { - let npm_roots = prior + let prior_roots = prior .filter(|p| p.taken_with(options)) - .map(|p| p.roots.as_slice()); - dispatch_find(partitioned, options, silent, merge_qualified, npm_roots).await + .map(|p| p.roots.clone()); + let npm_roots = match prior_roots { + Some(roots) => Some(roots), + None if partitioned + .get(&Ecosystem::Npm) + .is_some_and(|p| !p.is_empty()) => + { + NpmCrawler.get_node_modules_paths(options).await.ok() + } + None => None, + }; + let mut found = dispatch_find( + partitioned, + options, + silent, + merge_qualified, + npm_roots.as_deref(), + ) + .await; + if let Some(roots) = npm_roots { + found = tokio::task::spawn_blocking(move || { + add_npm_links_to_copies(&mut found, &roots); + found + }) + .await + .expect("npm link enumeration does not panic"); + } + found +} + +/// Add to each npm purl's `copies` every package link directly in one of +/// `roots` (scoped ones one level down) that resolves to one of its +/// copies. The crawler records a plain-name link at its own path +/// (`packages/a/node_modules/ms`), but never an alias link +/// (`packages/a/node_modules/lp -> …/.pnpm/left-pad@1.3.0/node_modules/left-pad`): +/// links are not physical copies, and apply and VEX must see each physical +/// copy once. Path scoping instead needs every path that installs the +/// package, so the alias link is added here (#1266). +fn add_npm_links_to_copies(copies: &mut HashMap>, roots: &[PathBuf]) { + let mut by_target: HashMap> = HashMap::new(); + for (purl, paths) in copies.iter() { + if !purl.starts_with("pkg:npm/") { + continue; + } + for path in paths { + if let Ok(target) = std::fs::canonicalize(path) { + by_target.entry(target).or_default().push(purl.clone()); + } + } + } + if by_target.is_empty() { + return; + } + let entries = |dir: &Path| -> Vec<(String, PathBuf, std::fs::FileType)> { + let Ok(read) = std::fs::read_dir(dir) else { + return Vec::new(); + }; + read.filter_map(Result::ok) + .filter_map(|e| { + let name = e.file_name().to_string_lossy().into_owned(); + let file_type = e.file_type().ok()?; + (!name.starts_with('.')).then(|| (name, e.path(), file_type)) + }) + .collect() + }; + for root in roots { + let mut links = Vec::new(); + for (name, path, file_type) in entries(root) { + if name.starts_with('@') && file_type.is_dir() { + links.extend( + entries(&path) + .into_iter() + .filter(|(_, _, ft)| ft.is_symlink()) + .map(|(_, p, _)| p), + ); + } else if file_type.is_symlink() { + links.push(path); + } + } + for link in links { + let Ok(target) = std::fs::canonicalize(&link) else { + continue; + }; + for purl in by_target.get(&target).into_iter().flatten() { + let paths = copies.get_mut(purl).expect("keyed from copies"); + if !paths.contains(&link) { + paths.push(link.clone()); + } + } + } + } } /// Qualified-aware PURL resolution for rollback, vendor, repair and diff --git a/crates/socket-patch-cli/tests/rollback/rollback_duality_invariants.rs b/crates/socket-patch-cli/tests/rollback/rollback_duality_invariants.rs index f58b0c3c3..875d29ae8 100644 --- a/crates/socket-patch-cli/tests/rollback/rollback_duality_invariants.rs +++ b/crates/socket-patch-cli/tests/rollback/rollback_duality_invariants.rs @@ -905,3 +905,97 @@ fn uuid_and_purl_targets_still_work() { assert!(m["patches"].get("pkg:npm/dual-b@1.0.0").is_none(), "{m}"); assert!(m["patches"].get("pkg:npm/dual-a@1.0.0").is_some(), "{m}"); } + +// --------------------------------------------------------------------------- +// #1266: a member's alias link into the root store is in the member's scope +// --------------------------------------------------------------------------- + +/// pnpm's isolated linker (and npm `install-strategy=linked`) put a +/// member's alias dependency (`"lp": "npm:path-alias-pkg@1.0.0"`) at +/// `packages/a/node_modules/lp`, a symlink into the root store. That link is +/// what member a loads, so `rollback packages/a` must restore the package, +/// exactly as it does for a plain-name link. +#[cfg(unix)] +#[test] +fn path_scoped_rollback_selects_member_alias_link() { + // pnpm's `.pnpm` store, then npm's linked `.store` (npm 10 names an + // alias's entry and dir after the alias). + for (store_entry, dir) in [ + (".pnpm/path-alias-pkg@1.0.0", "path-alias-pkg"), + (".store/lp@1.0.0-0a1b2c3d", "lp"), + ] { + assert_member_alias_link_rolled_back(store_entry, dir); + } +} + +#[cfg(unix)] +fn assert_member_alias_link_rolled_back(store_entry: &str, dir: &str) { + let before: &[u8] = b"path-alias-original\n"; + let after: &[u8] = b"path-alias-patched\n"; + let before_hash = git_sha256(before); + let after_hash = git_sha256(after); + let purl = "pkg:npm/path-alias-pkg@1.0.0"; + + let tmp = tempfile::tempdir().expect("tempdir"); + write_root_package_json(tmp.path()); + std::fs::write( + tmp.path().join("pnpm-workspace.yaml"), + "packages: ['packages/*']\n", + ) + .expect("write pnpm-workspace.yaml"); + let store_pkg = tmp + .path() + .join(format!("node_modules/{store_entry}/node_modules/{dir}")); + std::fs::create_dir_all(&store_pkg).expect("create store package dir"); + std::fs::write( + store_pkg.join("package.json"), + r#"{ "name": "path-alias-pkg", "version": "1.0.0" }"#, + ) + .expect("write package.json"); + std::fs::write(store_pkg.join("index.js"), after).expect("write index.js"); + let member_nm = tmp.path().join("packages/a/node_modules"); + std::fs::create_dir_all(&member_nm).expect("create member node_modules"); + std::fs::write( + tmp.path().join("packages/a/package.json"), + r#"{ "name": "a", "version": "1.0.0", "dependencies": { "lp": "npm:path-alias-pkg@1.0.0" } }"#, + ) + .expect("write member package.json"); + std::os::unix::fs::symlink( + format!("../../../node_modules/{store_entry}/node_modules/{dir}"), + member_nm.join("lp"), + ) + .expect("link member alias"); + let socket = write_socket_manifest( + tmp.path(), + &[manifest_entry( + purl, + "33333333-3333-4333-8333-333333333333", + &before_hash, + &after_hash, + )], + false, + ); + stage_blob(&socket, &before_hash, before); + stage_blob(&socket, &after_hash, after); + + let (code, stdout, stderr) = run(tmp.path(), &["--json", "--offline", "--yes", "packages/a"]); + assert_eq!( + code, 0, + "rollback of the member must select its alias link ({store_entry}); stdout=\n{stdout}\nstderr=\n{stderr}" + ); + let v: serde_json::Value = serde_json::from_str(&stdout).expect("valid JSON"); + assert_eq!(v["status"], "success", "stdout=\n{stdout}"); + assert_eq!( + crate::rollback_json::manifest_removed(&v), + serde_json::json!([purl]), + "the aliased entry is in scope; stdout=\n{stdout}" + ); + let content = std::fs::read(member_nm.join("lp/index.js")).expect("read alias file"); + assert_eq!( + git_sha256(&content), + before_hash, + "member a's alias restored" + ); + let content = std::fs::read(store_pkg.join("index.js")).expect("read store file"); + assert_eq!(git_sha256(&content), before_hash, "one physical copy"); +} diff --git a/crates/socket-patch-cli/tests/scan/scan_paths_e2e.rs b/crates/socket-patch-cli/tests/scan/scan_paths_e2e.rs index a0f7d7adc..23241aafd 100644 --- a/crates/socket-patch-cli/tests/scan/scan_paths_e2e.rs +++ b/crates/socket-patch-cli/tests/scan/scan_paths_e2e.rs @@ -886,3 +886,124 @@ async fn paths_scope_selects_every_nested_member_copy() { assert_eq!(v["scannedPackages"], 1, "scan {pattern}: {v}"); } } + +/// #1266: a workspace member's ALIAS dependency (`"lp": +/// "npm:left-pad@1.3.0"`) under a linked layout is a symlink +/// `packages/a/node_modules/lp` into the root store, just like its +/// plain-name link `packages/a/node_modules/ms`. Scoping to the member must +/// select the aliased package too, not only the plain-name one. Left-pad +/// lives at `node_modules///node_modules/`. +#[cfg(unix)] +async fn assert_member_alias_link_in_scope(store: &str, lp_entry: &str, lp_dir: &str) { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + std::fs::write( + root.join("package.json"), + r#"{ "name": "root", "version": "1.0.0", "private": true, "workspaces": ["packages/*"] }"#, + ) + .unwrap(); + std::fs::write( + root.join("pnpm-workspace.yaml"), + "packages: ['packages/*']\n", + ) + .unwrap(); + // The aliased package's store copy: its own package.json names the + // real package, whatever the entry and dir are called. + let lp_pkg = root + .join("node_modules") + .join(store) + .join(lp_entry) + .join("node_modules") + .join(lp_dir); + std::fs::create_dir_all(&lp_pkg).unwrap(); + std::fs::write( + lp_pkg.join("package.json"), + r#"{ "name": "left-pad", "version": "1.3.0" }"#, + ) + .unwrap(); + for (member, deps, links) in [ + ( + "a", + r#"{ "lp": "npm:left-pad@1.3.0", "ms": "2.1.2" }"#, + vec![ + ("lp", format!("{store}/{lp_entry}/node_modules/{lp_dir}")), + ("ms", format!("{store}/ms@2.1.2/node_modules/ms")), + ], + ), + ( + "b", + r#"{ "is-number": "7.0.0" }"#, + vec![( + "is-number", + format!("{store}/is-number@7.0.0/node_modules/is-number"), + )], + ), + ] { + let member_dir = root.join("packages").join(member); + std::fs::create_dir_all(member_dir.join("node_modules")).unwrap(); + std::fs::write( + member_dir.join("package.json"), + format!(r#"{{ "name": "{member}", "version": "1.0.0", "dependencies": {deps} }}"#), + ) + .unwrap(); + for (link, target) in links { + std::os::unix::fs::symlink( + format!("../../../node_modules/{target}"), + member_dir.join("node_modules").join(link), + ) + .unwrap(); + } + } + for (prefix, name, version) in [ + (format!("node_modules/{store}/ms@2.1.2"), "ms", "2.1.2"), + ( + format!("node_modules/{store}/is-number@7.0.0"), + "is-number", + "7.0.0", + ), + ] { + write_npm_package_at(root, &prefix, name, version); + } + + for pattern in ["packages/a", "packages/a/node_modules/lp"] { + let (bodies, v) = scoped_agent_scan(root, pattern).await; + assert_eq!( + bodies.len(), + 1, + "scan {pattern} must query the batch API once; envelope {v}" + ); + assert!( + bodies[0].contains("pkg:npm/left-pad@1.3.0"), + "scan {pattern} must select member a's aliased left-pad ({store}/{lp_entry}); body: {}", + bodies[0] + ); + assert!( + !bodies[0].contains("is-number"), + "scan {pattern} must not select member b's is-number; body: {}", + bodies[0] + ); + } + let (bodies, v) = scoped_agent_scan(root, "packages/a").await; + assert!( + bodies[0].contains("pkg:npm/ms@2.1.2"), + "scan packages/a must still select the plain-name link; body: {}", + bodies[0] + ); + assert_eq!(v["scannedPackages"], 2, "scan packages/a: {v}"); +} + +/// #1266, pnpm's default isolated linker. +#[cfg(unix)] +#[tokio::test] +async fn paths_scope_selects_pnpm_member_alias_link() { + assert_member_alias_link_in_scope(".pnpm", "left-pad@1.3.0", "left-pad").await; +} + +/// #1266, npm `install-strategy=linked`: npm 11+ names the store entry +/// after the real package, npm 10 after the alias. +#[cfg(unix)] +#[tokio::test] +async fn paths_scope_selects_npm_linked_member_alias_link() { + assert_member_alias_link_in_scope(".store", "left-pad@1.3.0-0a1b2c3d", "left-pad").await; + assert_member_alias_link_in_scope(".store", "lp@1.3.0-0a1b2c3d", "lp").await; +} From 84b3690169e62fbe9f5157c4e5a3b4f489c409fd Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 08:10:20 +0000 Subject: [PATCH 3/4] Keep alias links out of rollback's copy list The link pass lived in the resolver rollback also restores from, so every rollback did extra filesystem work it then threw away. It now runs only for scan and rollback path targets, in its own lookup. A member's alias link can also point at a pnpm peer variant the crawl skipped once it found the root's copy, so store peer variants now count when matching links. A failed link enumeration falls back to the plain copies instead of aborting the run. Assisted-by: Claude Code:claude-opus-5-5 --- .../socket-patch-cli/src/commands/rollback.rs | 3 +- .../socket-patch-cli/src/commands/scan/mod.rs | 5 +- .../src/ecosystem_dispatch.rs | 116 ++++++++++-------- .../tests/scan/scan_paths_e2e.rs | 73 +++++++++-- 4 files changed, 137 insertions(+), 60 deletions(-) diff --git a/crates/socket-patch-cli/src/commands/rollback.rs b/crates/socket-patch-cli/src/commands/rollback.rs index c4eb19534..6a480ff54 100644 --- a/crates/socket-patch-cli/src/commands/rollback.rs +++ b/crates/socket-patch-cli/src/commands/rollback.rs @@ -1591,10 +1591,11 @@ pub async fn run(args: RollbackArgs) -> i32 { global: args.common.global, global_prefix: args.common.global_prefix.clone(), }; - let discovered = find_all_packages_for_rollback( + let discovered = crate::ecosystem_dispatch::find_package_paths_for_path_scope( &partitioned, &crawler_options, args.common.silent || args.common.json, + None, ) .await; // One single-pattern scope per raw pattern, compiled once (not per diff --git a/crates/socket-patch-cli/src/commands/scan/mod.rs b/crates/socket-patch-cli/src/commands/scan/mod.rs index 019c1b29c..f49734e66 100644 --- a/crates/socket-patch-cli/src/commands/scan/mod.rs +++ b/crates/socket-patch-cli/src/commands/scan/mod.rs @@ -33,8 +33,7 @@ use std::path::{Path, PathBuf}; use crate::args::{apply_env_toggles, GlobalArgs}; use crate::commands::vex::{generate_vex_from_manifest_path, VexEmbedArgs}; use crate::ecosystem_dispatch::{ - crawl_ecosystems, crawl_ecosystems_with_npm, find_all_packages_for_rollback_reusing, - partition_purls, + crawl_ecosystems, crawl_ecosystems_with_npm, find_package_paths_for_path_scope, partition_purls, }; use crate::json_envelope::{ usage_error, Command as JsonCommand, Envelope, EnvelopeError, PatchAction, PatchEvent, @@ -2181,7 +2180,7 @@ async fn run_scan( unresolved.dedup(); if !unresolved.is_empty() { let partitioned = partition_purls(&unresolved, args.common.ecosystems.as_deref()); - let copies = find_all_packages_for_rollback_reusing( + let copies = find_package_paths_for_path_scope( &partitioned, &crawler_options, true, diff --git a/crates/socket-patch-cli/src/ecosystem_dispatch.rs b/crates/socket-patch-cli/src/ecosystem_dispatch.rs index edba73152..62d025ef3 100644 --- a/crates/socket-patch-cli/src/ecosystem_dispatch.rs +++ b/crates/socket-patch-cli/src/ecosystem_dispatch.rs @@ -634,33 +634,49 @@ pub async fn find_all_packages_for_rollback( } /// [`find_all_packages_for_rollback`], taking the npm `node_modules` roots -/// from `prior` as [`find_packages_for_rollback_reusing`] does. Used by -/// `scan`'s path scoping, which must see the same copies `rollback`'s path -/// targets do. -/// -/// An npm package's copies here also include each link in a `node_modules` -/// root that resolves to one of them ([`add_npm_links_to_copies`]): path -/// scoping asks which install paths reach a package, and pnpm's isolated -/// linker or npm's `install-strategy=linked` reach a workspace member's -/// ALIAS dependency only through such a link (#1266). +/// from `prior` as [`find_packages_for_rollback_reusing`] does. pub async fn find_all_packages_for_rollback_reusing( partitioned: &HashMap>, options: &CrawlerOptions, silent: bool, prior: Option<&NpmCrawlSnapshot>, ) -> HashMap> { - let prior_roots = prior + let npm_roots = prior .filter(|p| p.taken_with(options)) - .map(|p| p.roots.clone()); - let npm_roots = match prior_roots { - Some(roots) => Some(roots), - None if partitioned - .get(&Ecosystem::Npm) - .is_some_and(|p| !p.is_empty()) => - { - NpmCrawler.get_node_modules_paths(options).await.ok() - } - None => None, + .map(|p| p.roots.as_slice()); + dispatch_find(partitioned, options, silent, merge_qualified, npm_roots).await +} + +/// The install paths that reach each purl, for `scan`'s and `rollback`'s +/// path targets: [`find_all_packages_for_rollback_reusing`]'s physical +/// copies, plus each npm package link directly in a `node_modules` root +/// (scoped ones one level down) that resolves to one of them or to one +/// of their store peer variants. +/// +/// The crawler records a plain-name link at its own path +/// (`packages/a/node_modules/ms`), but never an alias link +/// (`packages/a/node_modules/lp -> …/.pnpm/left-pad@1.3.0/node_modules/left-pad`): +/// links are not physical copies, and apply and VEX must see each copy +/// once (#856). Path scoping asks which paths install the package +/// instead, and pnpm's isolated linker or npm's `install-strategy=linked` +/// reach a workspace member's ALIAS dependency only through such a link +/// (#1266). The member's link can resolve to a peer variant the crawl +/// skipped once it found the root's copy, so the variants count too. +/// The result is for path matching only: it can name one physical copy +/// more than once. +pub async fn find_package_paths_for_path_scope( + partitioned: &HashMap>, + options: &CrawlerOptions, + silent: bool, + prior: Option<&NpmCrawlSnapshot>, +) -> HashMap> { + let has_npm = partitioned + .get(&Ecosystem::Npm) + .is_some_and(|purls| !purls.is_empty()); + let npm_roots = match prior.and_then(|p| p.roots_for(options)) { + Some(roots) if has_npm => Some(roots.to_vec()), + None if has_npm => NpmCrawler.get_node_modules_paths(options).await.ok(), + _ => None, }; let mut found = dispatch_find( partitioned, @@ -670,40 +686,43 @@ pub async fn find_all_packages_for_rollback_reusing( npm_roots.as_deref(), ) .await; - if let Some(roots) = npm_roots { - found = tokio::task::spawn_blocking(move || { - add_npm_links_to_copies(&mut found, &roots); - found - }) - .await - .expect("npm link enumeration does not panic"); - } - found -} - -/// Add to each npm purl's `copies` every package link directly in one of -/// `roots` (scoped ones one level down) that resolves to one of its -/// copies. The crawler records a plain-name link at its own path -/// (`packages/a/node_modules/ms`), but never an alias link -/// (`packages/a/node_modules/lp -> …/.pnpm/left-pad@1.3.0/node_modules/left-pad`): -/// links are not physical copies, and apply and VEX must see each physical -/// copy once. Path scoping instead needs every path that installs the -/// package, so the alias link is added here (#1266). -fn add_npm_links_to_copies(copies: &mut HashMap>, roots: &[PathBuf]) { + let Some(roots) = npm_roots else { + return found; + }; let mut by_target: HashMap> = HashMap::new(); - for (purl, paths) in copies.iter() { - if !purl.starts_with("pkg:npm/") { + for (purl, paths) in &found { + if Ecosystem::from_purl(purl) != Some(Ecosystem::Npm) { continue; } - for path in paths { - if let Ok(target) = std::fs::canonicalize(path) { + for path in with_store_peer_variant_copies(paths.clone()).await { + if let Ok(target) = tokio::fs::canonicalize(&path).await { by_target.entry(target).or_default().push(purl.clone()); } } } if by_target.is_empty() { - return; + return found; } + // A failed enumeration only loses the extra links; the copies stand. + let links = tokio::task::spawn_blocking(move || npm_links_to(&by_target, &roots)) + .await + .unwrap_or_default(); + for (purl, link) in links { + let paths = found.entry(purl).or_default(); + if !paths.contains(&link) { + paths.push(link); + } + } + found +} + +/// Every package link directly in one of `roots` (scoped ones one level +/// down) that resolves to a key of `by_target`, paired with each purl +/// that key belongs to. +fn npm_links_to( + by_target: &HashMap>, + roots: &[PathBuf], +) -> Vec<(String, PathBuf)> { let entries = |dir: &Path| -> Vec<(String, PathBuf, std::fs::FileType)> { let Ok(read) = std::fs::read_dir(dir) else { return Vec::new(); @@ -716,6 +735,7 @@ fn add_npm_links_to_copies(copies: &mut HashMap>, roots: &[ }) .collect() }; + let mut out = Vec::new(); for root in roots { let mut links = Vec::new(); for (name, path, file_type) in entries(root) { @@ -735,13 +755,11 @@ fn add_npm_links_to_copies(copies: &mut HashMap>, roots: &[ continue; }; for purl in by_target.get(&target).into_iter().flatten() { - let paths = copies.get_mut(purl).expect("keyed from copies"); - if !paths.contains(&link) { - paths.push(link.clone()); - } + out.push((purl.clone(), link.clone())); } } } + out } /// Qualified-aware PURL resolution for rollback, vendor, repair and diff --git a/crates/socket-patch-cli/tests/scan/scan_paths_e2e.rs b/crates/socket-patch-cli/tests/scan/scan_paths_e2e.rs index 23241aafd..781a8de6b 100644 --- a/crates/socket-patch-cli/tests/scan/scan_paths_e2e.rs +++ b/crates/socket-patch-cli/tests/scan/scan_paths_e2e.rs @@ -982,14 +982,15 @@ async fn assert_member_alias_link_in_scope(store: &str, lp_entry: &str, lp_dir: "scan {pattern} must not select member b's is-number; body: {}", bodies[0] ); + if pattern == "packages/a" { + assert!( + bodies[0].contains("pkg:npm/ms@2.1.2"), + "scan packages/a must still select the plain-name link; body: {}", + bodies[0] + ); + assert_eq!(v["scannedPackages"], 2, "scan packages/a: {v}"); + } } - let (bodies, v) = scoped_agent_scan(root, "packages/a").await; - assert!( - bodies[0].contains("pkg:npm/ms@2.1.2"), - "scan packages/a must still select the plain-name link; body: {}", - bodies[0] - ); - assert_eq!(v["scannedPackages"], 2, "scan packages/a: {v}"); } /// #1266, pnpm's default isolated linker. @@ -1007,3 +1008,61 @@ async fn paths_scope_selects_npm_linked_member_alias_link() { assert_member_alias_link_in_scope(".store", "left-pad@1.3.0-0a1b2c3d", "left-pad").await; assert_member_alias_link_in_scope(".store", "lp@1.3.0-0a1b2c3d", "lp").await; } + +/// #1266: the root's own `left-pad` link resolves to one pnpm peer +/// variant, member a's alias `lp` to another. The crawl stops at the +/// root's copy and never lists member a's variant, yet member a loads it, +/// so scoping to member a must still select left-pad. +#[cfg(unix)] +#[tokio::test] +async fn paths_scope_selects_member_alias_link_to_a_peer_variant() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + std::fs::write( + root.join("package.json"), + r#"{ "name": "root", "version": "1.0.0", "private": true, "dependencies": { "left-pad": "1.3.0" } }"#, + ) + .unwrap(); + std::fs::write( + root.join("pnpm-workspace.yaml"), + "packages: ['packages/*']\n", + ) + .unwrap(); + for variant in ["left-pad@1.3.0_react@17.0.2", "left-pad@1.3.0_react@18.2.0"] { + write_npm_package_at( + root, + &format!("node_modules/.pnpm/{variant}"), + "left-pad", + "1.3.0", + ); + } + std::os::unix::fs::symlink( + ".pnpm/left-pad@1.3.0_react@17.0.2/node_modules/left-pad", + root.join("node_modules/left-pad"), + ) + .unwrap(); + let member_dir = root.join("packages/a"); + std::fs::create_dir_all(member_dir.join("node_modules")).unwrap(); + std::fs::write( + member_dir.join("package.json"), + r#"{ "name": "a", "version": "1.0.0", "dependencies": { "lp": "npm:left-pad@1.3.0" } }"#, + ) + .unwrap(); + std::os::unix::fs::symlink( + "../../../node_modules/.pnpm/left-pad@1.3.0_react@18.2.0/node_modules/left-pad", + member_dir.join("node_modules/lp"), + ) + .unwrap(); + + let (bodies, v) = scoped_agent_scan(root, "packages/a").await; + assert_eq!( + bodies.len(), + 1, + "scan packages/a must query the batch API once; envelope {v}" + ); + assert!( + bodies[0].contains("pkg:npm/left-pad@1.3.0"), + "scan packages/a must select the peer variant member a's alias loads; body: {}", + bodies[0] + ); +} From 4288a72bbd18bb6e59cc7eb620b2a37ad7dabbc9 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 10:18:32 +0000 Subject: [PATCH 4/4] Keep scoped scans fast on large pnpm stores The previous commit matched member links by expanding every crawled npm purl's store peer variants, re-listing the whole store once per purl. A path-scoped scan of a 3,000-entry pnpm store went from 0.24s to 35s (debug build). Links in each node_modules root are now matched by their target's own package.json name@version, in one pass however many purls are asked for. The same 3,000-entry scan takes 0.24s again, and a link to a peer variant the crawl skipped still counts. Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-cli/CLI_CONTRACT.md | 2 +- .../src/ecosystem_dispatch.rs | 76 +++++++++++-------- 2 files changed, 44 insertions(+), 34 deletions(-) diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index b51148203..28346a42f 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -210,7 +210,7 @@ For a **9.0 root lock**, the CLI ensures `pnpm-workspace.yaml` carries `trustLoc **Path-scoped scans (`scan [PATHS]...`, v5.0)**: what a PATH means depends on the mode. * **Hosted and vendored mode (bare `scan` included) — project directories** (`run_project_dirs`). Each PATH is a directory, or a glob (`*?[`) matching directories, relative to `--cwd`; the set is sorted and deduplicated, and each directory is scanned on its own exactly as if it were `--cwd` (its own lockfiles, ledgers and `.socket/`). With more than one directory, each run is headed `== ==` on stdout (unless `--silent`), and the exit code is the worst of the runs. Usage errors (exit 2, stderr only, before any scan): a PATH that is not a directory (`` `X` is not a directory``), a glob matching no directory (`` `X` matches no directory``), an invalid glob, and `--json` with more than one directory (`--json takes one project directory (N given); run one scan per directory`), so stdout stays one document. Likewise `--vex` with more than one directory (`--vex takes one project directory (N given); run one scan per directory`): the one output path would be overwritten by each run. -* **Agent mode (and a mode-less `--prune`/`--global` report) — installed-path globs** scoping DISCOVERY at the **purl level**: a package is in scope iff ANY of its crawled installed copies sits under a matching path (every copy, enumerated the way `rollback`'s path targets enumerate them: a pnpm workspace member's link into the root `.pnpm` store (or npm's linked `.store`), including an alias link such as `packages/a/node_modules/lp` for `"lp": "npm:left-pad@1.3.0"`, and a nested member copy count, not just the one copy the crawl records per purl), and a selected package is then handled with ALL its copies (scoping selects which packages are considered, never which copies). Glob semantics (shared with `rollback`'s path targets, `src/path_scope.rs`): Unix-shell globs with `require_literal_separator` — `*`/`?` never cross a `/`, `**` spans directories; a pattern matching any **ancestor** directory of the copy path also matches, so a bare `scan packages/foo` scopes the whole subtree without `/**`; relative patterns match against the copy path relativized to `--cwd`, absolute patterns against the absolute path (the ONLY way to reach paths outside the project tree, e.g. `--global` stores — a relative pattern never matches outside `--cwd`); leading `./` and trailing `/` are normalized away, matching is purely textual (no filesystem access or symlink resolution), case-sensitive except on Windows (whose filesystems are not); an unparseable or empty pattern is a usage error (exit 2). **The prune universe is never narrowed**: the path filter is applied strictly AFTER the `scanned_purls` capture (and after `--ecosystems`), so `scan PATHS --prune` prunes exactly what an unscoped `scan --prune` would — a scoped scan can never treat an out-of-scope package as uninstalled (the same fail-safe as the `--ecosystems` filter). Lockfile-only and vendor-ledger supplement records have no installed path and are EXCLUDED from a path-scoped scan, surfaced as one run-level `path_scope_excluded_supplements` warning carrying the count. A scope matching nothing is a normal empty scan — exit 0, zero packages, **no GC** (the zero-package early return fires before any GC). `PATHS` combine with `--mode agent`/`--sync`/`--prune`/`--global`. Every scan JSON envelope past the argument checks (success, zero-package, all-batches-failed) carries a `paths` key echoing the patterns verbatim (empty array when unscoped; a hosted/vendored per-directory run is unscoped, so it is `[]`). One-sentence duality rule: **a target that selects nothing is an error on `rollback` (exit 1) and an empty scan on an agent-mode `scan` (exit 0)**. +* **Agent mode (and a mode-less `--prune`/`--global` report) — installed-path globs** scoping DISCOVERY at the **purl level**: a package is in scope iff ANY of its crawled installed copies sits under a matching path (every copy, enumerated the way `rollback`'s path targets enumerate them: any package link in a `node_modules` root, such as a pnpm workspace member's link into the root `.pnpm` store (or npm's linked `.store`), alias links included (`packages/a/node_modules/lp` for `"lp": "npm:left-pad@1.3.0"`, matched by its target's own `package.json` name@version), and a nested member copy count, not just the one copy the crawl records per purl), and a selected package is then handled with ALL its copies (scoping selects which packages are considered, never which copies). Glob semantics (shared with `rollback`'s path targets, `src/path_scope.rs`): Unix-shell globs with `require_literal_separator` — `*`/`?` never cross a `/`, `**` spans directories; a pattern matching any **ancestor** directory of the copy path also matches, so a bare `scan packages/foo` scopes the whole subtree without `/**`; relative patterns match against the copy path relativized to `--cwd`, absolute patterns against the absolute path (the ONLY way to reach paths outside the project tree, e.g. `--global` stores — a relative pattern never matches outside `--cwd`); leading `./` and trailing `/` are normalized away, matching is purely textual (no filesystem access or symlink resolution), case-sensitive except on Windows (whose filesystems are not); an unparseable or empty pattern is a usage error (exit 2). **The prune universe is never narrowed**: the path filter is applied strictly AFTER the `scanned_purls` capture (and after `--ecosystems`), so `scan PATHS --prune` prunes exactly what an unscoped `scan --prune` would — a scoped scan can never treat an out-of-scope package as uninstalled (the same fail-safe as the `--ecosystems` filter). Lockfile-only and vendor-ledger supplement records have no installed path and are EXCLUDED from a path-scoped scan, surfaced as one run-level `path_scope_excluded_supplements` warning carrying the count. A scope matching nothing is a normal empty scan — exit 0, zero packages, **no GC** (the zero-package early return fires before any GC). `PATHS` combine with `--mode agent`/`--sync`/`--prune`/`--global`. Every scan JSON envelope past the argument checks (success, zero-package, all-batches-failed) carries a `paths` key echoing the patterns verbatim (empty array when unscoped; a hosted/vendored per-directory run is unscoped, so it is `[]`). One-sentence duality rule: **a target that selects nothing is an error on `rollback` (exit 1) and an empty scan on an agent-mode `scan` (exit 0)**. `scan --mode vendored` swaps the in-place apply for the vendor pipeline: discover → download the selected patch records **into memory** (no manifest write) → vendor every selected dependency via the same engine as the `vendor` command (under the same lock). Vendored mode is **manifest-free (v5.0)**: `.socket/manifest.json` is never written or read by a vendored run; each ledger entry carries `detached: true` plus an embedded copy of the patch record (`record`) as its verification source, and the run's footprint is `.socket/vendor/**` only. The vendor step's scope is what discovery selected — the former "whole manifest is vendored" re-vendor on an empty discovery is retired (`repair` verifies and redownloads committed vendored state; `scan --prune` reconciles ledger entries whose dependency left the lockfile). The vendor-ledger discovery supplement (the fresh-clone rule: a ledger entry with no installed copy stays discoverable because its committed artifact IS the dependency) holds only while the lockfile still resolves through that artifact: an entry the lockfile in-use probe (the one `--prune` reverts by) proves unwired, because the dependency was upgraded or removed, is NOT discovered and so is never re-vendored. A run without a non-hosted `--prune` reports it through the run-level `vendor_ledger_entry_unwired` warning; a `--prune` run reverts it in its GC and exits 0. That GC runs even when the crawl found no packages, as its vendored half alone (the manifest prune stays skipped there). A package the ledger holds at an older patch uuid is still **re-vendored automatically** when discovery selects the newer patch (its old uuid dir is removed — `vendor_stale_artifact_removed`) — unless the patch service has no prebuilt artifact for the newer patch yet (or at all): an npm-family package then keeps the older patch and is a `skipped` `vendor_prebuilt_pending` / `vendor_prebuilt_unavailable` event, not a failure (#954); same-uuid re-runs reuse the embedded record, skip the patch-view fetch, and are `already_vendored` skips. **Legacy manifest-mode entries**: when a vendored run vendors a purl that also has a `.socket/manifest.json` record (a project vendored by a pre-5.0 binary, or by standalone `vendor` from an agent-mode manifest), that manifest record is dropped in the same run — the ledger becomes the owner (migration write); an emptied manifest is left as `{"patches": {}}`, never deleted. The migration is reported through the run-level `warnings[]` (stderr in human mode), never as a run error: `vendor_manifest_record_migrated` (`N manifest records moved to the vendor ledger (vendored mode is manifest-free): `) or `vendor_manifest_migration_failed` (the manifest or the ledger could not be read or rewritten; the legacy records were left in place) — so a corrupt `.socket/manifest.json` no longer fails a vendored run (standalone `vendor`, the one manifest-driven writer, still fails closed on it). With `--prune`, GC runs **after** the vendor step (the step never reads the manifest, and running the sweep last lets it reclaim what the run itself orphaned — a migrated legacy record's blobs, a superseded uuid dir). Under `--json` (v5.0, MAJOR) the download phase and the vendor engine record into the ONE scan envelope (no nested `download` / `vendor` objects): a fetched patch is a `downloaded` event (the patch metadata in `details`, plus `details.oldUuid` when the ledger holds the purl at another uuid — the re-vendor the vendor step then performs — whose human `[fetch]` line reads ` (replacing )`), a refused or failed one a `failed` event with its code, a patch the ledger already holds at the selected uuid no download event at all (reused, no fetch); then the vendor engine's events (`applied`, `rebuilt`, `skipped`, `failed`, `removed`, its uncounted advisories) follow with their warnings and sidecars. Every one of these events carries `details.mode: "vendored"`. Patch blobs are held in memory (see "Patch sources stay in memory" under the vendor contract). `--dry-run` previews per patch: `verified` (to vendor; `oldUuid` on a re-vendor), `skipped` / `already_vendored` (in sync), or `skipped` with the refusal code for an npm purl the wet run's Bun preflight (see the `get --mode vendored` bullet below) would refuse, or whose hosted pnpm pin the takeover would be refused on (see "Takeover reconciliation") — without network downloads or disk writes; the preview never flips status or exit (the human path — `scan` and `get` alike, through one shared printer — prints `[would-refuse] (): ` lines behind the `--silent` gate). Interactive mode prompts "Download and vendor N patches?" (singular for one). diff --git a/crates/socket-patch-cli/src/ecosystem_dispatch.rs b/crates/socket-patch-cli/src/ecosystem_dispatch.rs index 62d025ef3..d516c0341 100644 --- a/crates/socket-patch-cli/src/ecosystem_dispatch.rs +++ b/crates/socket-patch-cli/src/ecosystem_dispatch.rs @@ -9,7 +9,7 @@ use std::path::{Path, PathBuf}; use crate::args::GlobalArgs; use socket_patch_core::crawlers::npm_crawler::{ - retain_live_store_copies, with_store_peer_variant_copies, + build_npm_purl, parse_package_name, retain_live_store_copies, with_store_peer_variant_copies, }; use socket_patch_core::crawlers::walk_pool; use socket_patch_core::crawlers::CargoCrawler; @@ -650,8 +650,8 @@ pub async fn find_all_packages_for_rollback_reusing( /// The install paths that reach each purl, for `scan`'s and `rollback`'s /// path targets: [`find_all_packages_for_rollback_reusing`]'s physical /// copies, plus each npm package link directly in a `node_modules` root -/// (scoped ones one level down) that resolves to one of them or to one -/// of their store peer variants. +/// (scoped ones one level down) whose target's `package.json` names one +/// of the npm purls asked for. /// /// The crawler records a plain-name link at its own path /// (`packages/a/node_modules/ms`), but never an alias link @@ -660,23 +660,28 @@ pub async fn find_all_packages_for_rollback_reusing( /// once (#856). Path scoping asks which paths install the package /// instead, and pnpm's isolated linker or npm's `install-strategy=linked` /// reach a workspace member's ALIAS dependency only through such a link -/// (#1266). The member's link can resolve to a peer variant the crawl -/// skipped once it found the root's copy, so the variants count too. -/// The result is for path matching only: it can name one physical copy -/// more than once. +/// (#1266). A link is matched by its target's own identity, not against +/// the crawled copies, so a link to a store peer variant the crawl +/// skipped still counts, and the cost is one pass over the root links +/// however many purls are asked for. The result is for path matching +/// only: it can name one physical copy more than once. pub async fn find_package_paths_for_path_scope( partitioned: &HashMap>, options: &CrawlerOptions, silent: bool, prior: Option<&NpmCrawlSnapshot>, ) -> HashMap> { - let has_npm = partitioned - .get(&Ecosystem::Npm) - .is_some_and(|purls| !purls.is_empty()); + let mut wanted: HashMap> = HashMap::new(); + for purl in partitioned.get(&Ecosystem::Npm).into_iter().flatten() { + wanted + .entry(PurlKey::new(purl)) + .or_default() + .push(purl.clone()); + } let npm_roots = match prior.and_then(|p| p.roots_for(options)) { - Some(roots) if has_npm => Some(roots.to_vec()), - None if has_npm => NpmCrawler.get_node_modules_paths(options).await.ok(), - _ => None, + _ if wanted.is_empty() => None, + Some(roots) => Some(roots.to_vec()), + None => NpmCrawler.get_node_modules_paths(options).await.ok(), }; let mut found = dispatch_find( partitioned, @@ -689,22 +694,8 @@ pub async fn find_package_paths_for_path_scope( let Some(roots) = npm_roots else { return found; }; - let mut by_target: HashMap> = HashMap::new(); - for (purl, paths) in &found { - if Ecosystem::from_purl(purl) != Some(Ecosystem::Npm) { - continue; - } - for path in with_store_peer_variant_copies(paths.clone()).await { - if let Ok(target) = tokio::fs::canonicalize(&path).await { - by_target.entry(target).or_default().push(purl.clone()); - } - } - } - if by_target.is_empty() { - return found; - } // A failed enumeration only loses the extra links; the copies stand. - let links = tokio::task::spawn_blocking(move || npm_links_to(&by_target, &roots)) + let links = tokio::task::spawn_blocking(move || npm_links_to(&wanted, &roots)) .await .unwrap_or_default(); for (purl, link) in links { @@ -717,10 +708,10 @@ pub async fn find_package_paths_for_path_scope( } /// Every package link directly in one of `roots` (scoped ones one level -/// down) that resolves to a key of `by_target`, paired with each purl -/// that key belongs to. +/// down) whose target directory's `package.json` `name@version` is a key +/// of `wanted`, paired with each purl spelling that key stands for. fn npm_links_to( - by_target: &HashMap>, + wanted: &HashMap>, roots: &[PathBuf], ) -> Vec<(String, PathBuf)> { let entries = |dir: &Path| -> Vec<(String, PathBuf, std::fs::FileType)> { @@ -735,6 +726,25 @@ fn npm_links_to( }) .collect() }; + let identity = |link: &Path| -> Option { + let target = std::fs::canonicalize(link).ok()?; + let text = + socket_patch_core::utils::fs::read_regular_to_string_sync(&target.join("package.json")) + .ok()?; + let pkg: serde_json::Value = + serde_json::from_str(socket_patch_core::formats::text::strip_bom(&text)).ok()?; + let name = pkg.get("name")?.as_str()?; + let version = pkg.get("version")?.as_str()?; + if name.is_empty() || version.is_empty() { + return None; + } + let (namespace, leaf) = parse_package_name(name); + Some(PurlKey::new(&build_npm_purl( + namespace.as_deref(), + &leaf, + version, + ))) + }; let mut out = Vec::new(); for root in roots { let mut links = Vec::new(); @@ -751,10 +761,10 @@ fn npm_links_to( } } for link in links { - let Ok(target) = std::fs::canonicalize(&link) else { + let Some(key) = identity(&link) else { continue; }; - for purl in by_target.get(&target).into_iter().flatten() { + for purl in wanted.get(&key).into_iter().flatten() { out.push((purl.clone(), link.clone())); } }