From b3b9fa4ffbb2bb3d5242a5b673f420e82dbfca8a Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Fri, 14 Aug 2026 06:51:36 -0700 Subject: [PATCH] fix(npm): detect pnpm node-linker=pnp trees as pnpm MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pnpm's own PnP mode (node-linker=pnp in .npmrc) writes a .pnp.cjs loader at the project root just like yarn-berry does, but keeps real package directories in the pnpm virtual store — exactly the layout the CoW guard patches natively. The detector returned YarnBerryPnP for ANY PnP marker, so `apply` refused outright and told the user to run `yarn patch` in a pnpm repo; the vendored probe had the same hole one layer down and refused with vendor_yarn_berry_unsupported. Add a shared carve-out (pnpm_pnp_layout): a PnP marker with pnpm-lock.yaml, an installed pnpm store, and NO yarn.lock classifies as pnpm, so `apply` proceeds under the CoW guard. Anything ambiguous (yarn.lock alongside, or a stale pnpm-lock.yaml with no installed store) keeps the fail-closed yarn-berry refusal. Vendored mode still refuses on such trees — the file: rewiring has no fixtures under pnpm's PnP linker — but now with its own code (vendor_pnpm_pnp_unsupported) and a pnpm remedy: use `scan --mode hosted` or switch node-linker and reinstall. Un-ignores the RED test pnpm_pnp_mode_is_pnpm_not_yarn_berry and adds vendored-probe twins for the pnpm refusal and both ambiguity guards. Co-authored-by: Claude Fable 5 --- .../src/crawlers/pkg_managers.rs | 46 +++++++++- .../src/vendor/npm_flavor.rs | 84 ++++++++++++++++++- 2 files changed, 122 insertions(+), 8 deletions(-) diff --git a/crates/socket-patch-core/src/crawlers/pkg_managers.rs b/crates/socket-patch-core/src/crawlers/pkg_managers.rs index 896dea1c..52b872b2 100644 --- a/crates/socket-patch-core/src/crawlers/pkg_managers.rs +++ b/crates/socket-patch-core/src/crawlers/pkg_managers.rs @@ -59,7 +59,10 @@ pub enum NpmPkgManager { /// /// Precedence (first match wins): /// -/// 1. `.pnp.cjs`, `.pnp.js`, or `.pnp.loader.mjs` → yarn-berry PnP. +/// 1. `.pnp.cjs`, `.pnp.js`, or `.pnp.loader.mjs` → yarn-berry PnP — +/// unless the tree is pnpm's own `node-linker=pnp` layout (see +/// [`pnpm_pnp_layout`]), which also writes a `.pnp.cjs` but keeps +/// real package dirs in the pnpm virtual store → pnpm. /// 2. `bun.lock` or `bun.lockb` (+ `node_modules/`) → bun. /// 3. `node_modules/.modules.yaml` or `node_modules/.pnpm/` → pnpm. /// 4. `yarn.lock` (without PnP markers) + `node_modules/` → yarn classic. @@ -83,6 +86,16 @@ pub fn detect_npm_pkg_manager(project_root: &Path) -> NpmPkgManager { .iter() .any(|m| project_root.join(m).is_file()) { + // Carve-out: pnpm has its OWN PnP mode (`node-linker=pnp` in + // `.npmrc`) which also writes a `.pnp.cjs` loader at the root + // — but unlike yarn-berry the packages are real directories in + // the pnpm virtual store, exactly the layout the CoW guard + // patches natively. Reclassify as pnpm only on strong + // evidence; anything ambiguous stays the fail-closed + // yarn-berry refusal. + if pnpm_pnp_layout(project_root) { + return NpmPkgManager::Pnpm; + } return NpmPkgManager::YarnBerryPnP; } @@ -118,6 +131,34 @@ pub fn detect_npm_pkg_manager(project_root: &Path) -> NpmPkgManager { NpmPkgManager::Unknown } +/// Is a PnP-marker-bearing project root actually pnpm's own PnP mode +/// (`node-linker=pnp` in `.npmrc`) rather than yarn-berry? +/// +/// pnpm's PnP mode writes a `.pnp.cjs` loader at the root just like +/// yarn-berry does, but keeps real package directories in the pnpm +/// virtual store (`node_modules/.pnpm/@/node_modules/`, +/// verified against a real `pnpm install` with pnpm 10.28.2). The +/// reclassification requires ALL of: +/// +/// * an installed pnpm store (`node_modules/.modules.yaml` or +/// `node_modules/.pnpm/`) — a bare `pnpm-lock.yaml` left behind in a +/// yarn-berry repo must not escape the refusal; +/// * `pnpm-lock.yaml` at the root — an installed store without pnpm's +/// lockfile is not attributable to pnpm's PnP mode; +/// * NO `yarn.lock` — a tree carrying both lockfiles alongside the +/// loader is ambiguous (mid-migration multi-PM repo), so the +/// safety-critical yarn-berry refusal still wins. +/// +/// Shared with the vendor-side flavor probe +/// (`crate::vendor::npm_flavor::detect_npm_lock_flavor`) so both +/// detection sites agree on what counts as a pnpm-PnP tree. +pub(crate) fn pnpm_pnp_layout(project_root: &Path) -> bool { + let node_modules = project_root.join("node_modules"); + (node_modules.join(".modules.yaml").is_file() || node_modules.join(".pnpm").is_dir()) + && project_root.join("pnpm-lock.yaml").is_file() + && !project_root.join("yarn.lock").is_file() +} + #[cfg(test)] mod tests { use super::*; @@ -382,9 +423,6 @@ mod tests { /// /// Layout verified against a real `pnpm install` (pnpm 10.28.2). #[test] - #[ignore = "RED: documents a real bug — detect_npm_pkg_manager misreports a \ - pnpm `node-linker=pnp` tree as yarn berry. The test is correct; \ - the detector fix was not part of this change."] fn pnpm_pnp_mode_is_pnpm_not_yarn_berry() { let d = tempfile::tempdir().unwrap(); // Root markers emitted by `pnpm install` with node-linker=pnp. diff --git a/crates/socket-patch-core/src/vendor/npm_flavor.rs b/crates/socket-patch-core/src/vendor/npm_flavor.rs index d601e1ce..a8cd0c86 100644 --- a/crates/socket-patch-core/src/vendor/npm_flavor.rs +++ b/crates/socket-patch-core/src/vendor/npm_flavor.rs @@ -5,8 +5,11 @@ //! a `pnpm-lock.yaml` only routes to the pnpm backend when its //! `lockfileVersion` is one we have fixtures for, and a `yarn.lock` routes //! to classic or berry by its header (the v1 comment vs a top-level -//! `__metadata:` key). Only yarn PnP projects (`.pnp.*` loaders) are -//! refused outright — their packages never land on disk to stage. +//! `__metadata:` key). Only PnP projects (`.pnp.*` loaders) are refused +//! outright: yarn-berry PnP because its packages never land on disk to +//! stage, and pnpm's own `node-linker=pnp` mode (same loader file, real +//! package dirs) because the file: rewiring is unvalidated under that +//! linker — each with its own code and remedy. //! //! The router fans `vendor`/`revert` out per detected flavor. All five //! flavors have real backends: package-lock ([`super::npm_lock`]), @@ -83,7 +86,11 @@ const LOCKFILE_FAMILIES: [(NpmLockFlavor, &[&str]); 4] = [ /// Probe the project root for the lockfile flavor that drives npm installs. /// /// Decision table, first match wins: -/// 1. a PnP loader file → Err `vendor_yarn_berry_unsupported`; +/// 1. a PnP loader file → Err `vendor_yarn_berry_unsupported` — unless the +/// tree is pnpm's own `node-linker=pnp` layout (pnpm-lock.yaml + installed +/// pnpm store + no yarn.lock, see +/// [`crate::crawlers::pkg_managers::pnpm_pnp_layout`]) → Err +/// `vendor_pnpm_pnp_unsupported` with a pnpm remedy; /// 2. `bun.lock` → Bun; else `bun.lockb` → Err `vendor_bun_lockb_unsupported`; /// 3. `pnpm-lock.yaml` → head-sniff `lockfileVersion` (only `'9.0'`) → Pnpm, /// else Err `vendor_lockfile_version_unsupported`; @@ -109,9 +116,28 @@ pub(crate) async fn detect_npm_lock_flavor( }; // 1. Yarn berry PnP — checked first because it means packages are not on - // disk at all, whatever lockfiles are also lying around. + // disk at all, whatever lockfiles are also lying around. Carve-out: + // pnpm's own PnP mode (`node-linker=pnp` in `.npmrc`) writes the same + // loader but is a pnpm project — recommending `yarn patch` there is + // wrong twice over. Vendor still refuses (the file: rewiring has no + // fixtures under pnpm's PnP linker — fail closed), but with a pnpm + // diagnosis and remedy. for marker in PNP_MARKERS { if exists(marker).await { + if crate::crawlers::pkg_managers::pnpm_pnp_layout(project_root) { + return Err(( + "vendor_pnpm_pnp_unsupported", + format!( + "found `{marker}` alongside pnpm-lock.yaml and an installed pnpm \ + store: this is a pnpm project using `node-linker=pnp` (.npmrc), \ + not yarn berry — vendor's relative file: rewiring is not \ + validated under pnpm's Plug'n'Play linker; use `socket-patch \ + scan --mode hosted` (which edits pnpm-lock.yaml in place), or \ + switch .npmrc to `node-linker=isolated`, run `pnpm install`, \ + and re-run vendor" + ), + )); + } return Err(( "vendor_yarn_berry_unsupported", format!( @@ -458,6 +484,56 @@ mod tests { } } + /// Stage the root markers a real `pnpm install` with + /// `node-linker=pnp` emits (layout verified against pnpm 10.28.2): + /// the `.pnp.cjs` loader, pnpm-lock.yaml, and an installed virtual + /// store with the per-project pnpm marker. + async fn stage_pnpm_pnp_layout(root: &Path) { + touch(root, ".pnp.cjs", "/* pnp */").await; + touch(root, "pnpm-lock.yaml", PNPM_9).await; + tokio::fs::create_dir_all( + root.join("node_modules/.pnpm/flatted@3.3.1/node_modules/flatted"), + ) + .await + .unwrap(); + touch(&root.join("node_modules"), ".modules.yaml", "").await; + } + + /// pnpm's own PnP mode (`node-linker=pnp`) writes a `.pnp.cjs` just + /// like yarn-berry — the probe must refuse with a pnpm diagnosis and + /// remedy, never `vendor_yarn_berry_unsupported` / "use yarn patch" + /// (a yarn command in a pnpm repo). + #[tokio::test] + async fn pnpm_pnp_layout_refuses_with_pnpm_remedy() { + let tmp = tempfile::tempdir().unwrap(); + stage_pnpm_pnp_layout(tmp.path()).await; + let (code, detail) = detect_npm_lock_flavor(tmp.path()).await.unwrap_err(); + assert_eq!(code, "vendor_pnpm_pnp_unsupported"); + assert!(detail.contains("node-linker=pnp"), "{detail}"); + assert!(detail.contains("scan --mode hosted"), "{detail}"); + assert!(!detail.contains("yarn patch"), "{detail}"); + } + + /// The pnpm-PnP carve-out stays fail-closed: a yarn.lock alongside + /// the loader (ambiguous multi-PM tree), or a stale pnpm-lock.yaml + /// with no installed pnpm store, keeps the yarn-berry refusal. + #[tokio::test] + async fn pnpm_pnp_carve_out_stays_yarn_berry_when_ambiguous() { + // Both lockfiles present: ambiguous, yarn-berry refusal wins. + let tmp = tempfile::tempdir().unwrap(); + stage_pnpm_pnp_layout(tmp.path()).await; + touch(tmp.path(), "yarn.lock", YARN_BERRY).await; + let (code, _) = detect_npm_lock_flavor(tmp.path()).await.unwrap_err(); + assert_eq!(code, "vendor_yarn_berry_unsupported"); + + // Stale pnpm-lock.yaml, no installed store: no escape either. + let tmp = tempfile::tempdir().unwrap(); + touch(tmp.path(), ".pnp.cjs", "/* pnp */").await; + touch(tmp.path(), "pnpm-lock.yaml", PNPM_9).await; + let (code, _) = detect_npm_lock_flavor(tmp.path()).await.unwrap_err(); + assert_eq!(code, "vendor_yarn_berry_unsupported"); + } + #[tokio::test] async fn bun_lock_routes_and_lockb_refuses() { let tmp = tempfile::tempdir().unwrap();