Skip to content

Commit b3b9fa4

Browse files
mikolalysenkoclaude
mikolalysenko
and
claude
committed
fix(npm): detect pnpm node-linker=pnp trees as pnpm
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 <noreply@anthropic.com>
1 parent 4e5288e commit b3b9fa4

2 files changed

Lines changed: 122 additions & 8 deletions

File tree

‎crates/socket-patch-core/src/crawlers/pkg_managers.rs‎

Lines changed: 42 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,10 @@ pub enum NpmPkgManager {
5959
///
6060
/// Precedence (first match wins):
6161
///
62-
/// 1. `.pnp.cjs`, `.pnp.js`, or `.pnp.loader.mjs` → yarn-berry PnP.
62+
/// 1. `.pnp.cjs`, `.pnp.js`, or `.pnp.loader.mjs` → yarn-berry PnP —
63+
/// unless the tree is pnpm's own `node-linker=pnp` layout (see
64+
/// [`pnpm_pnp_layout`]), which also writes a `.pnp.cjs` but keeps
65+
/// real package dirs in the pnpm virtual store → pnpm.
6366
/// 2. `bun.lock` or `bun.lockb` (+ `node_modules/`) → bun.
6467
/// 3. `node_modules/.modules.yaml` or `node_modules/.pnpm/` → pnpm.
6568
/// 4. `yarn.lock` (without PnP markers) + `node_modules/` → yarn classic.
@@ -83,6 +86,16 @@ pub fn detect_npm_pkg_manager(project_root: &Path) -> NpmPkgManager {
8386
.iter()
8487
.any(|m| project_root.join(m).is_file())
8588
{
89+
// Carve-out: pnpm has its OWN PnP mode (`node-linker=pnp` in
90+
// `.npmrc`) which also writes a `.pnp.cjs` loader at the root
91+
// — but unlike yarn-berry the packages are real directories in
92+
// the pnpm virtual store, exactly the layout the CoW guard
93+
// patches natively. Reclassify as pnpm only on strong
94+
// evidence; anything ambiguous stays the fail-closed
95+
// yarn-berry refusal.
96+
if pnpm_pnp_layout(project_root) {
97+
return NpmPkgManager::Pnpm;
98+
}
8699
return NpmPkgManager::YarnBerryPnP;
87100
}
88101

@@ -118,6 +131,34 @@ pub fn detect_npm_pkg_manager(project_root: &Path) -> NpmPkgManager {
118131
NpmPkgManager::Unknown
119132
}
120133

134+
/// Is a PnP-marker-bearing project root actually pnpm's own PnP mode
135+
/// (`node-linker=pnp` in `.npmrc`) rather than yarn-berry?
136+
///
137+
/// pnpm's PnP mode writes a `.pnp.cjs` loader at the root just like
138+
/// yarn-berry does, but keeps real package directories in the pnpm
139+
/// virtual store (`node_modules/.pnpm/<name>@<ver>/node_modules/<name>`,
140+
/// verified against a real `pnpm install` with pnpm 10.28.2). The
141+
/// reclassification requires ALL of:
142+
///
143+
/// * an installed pnpm store (`node_modules/.modules.yaml` or
144+
/// `node_modules/.pnpm/`) — a bare `pnpm-lock.yaml` left behind in a
145+
/// yarn-berry repo must not escape the refusal;
146+
/// * `pnpm-lock.yaml` at the root — an installed store without pnpm's
147+
/// lockfile is not attributable to pnpm's PnP mode;
148+
/// * NO `yarn.lock` — a tree carrying both lockfiles alongside the
149+
/// loader is ambiguous (mid-migration multi-PM repo), so the
150+
/// safety-critical yarn-berry refusal still wins.
151+
///
152+
/// Shared with the vendor-side flavor probe
153+
/// (`crate::vendor::npm_flavor::detect_npm_lock_flavor`) so both
154+
/// detection sites agree on what counts as a pnpm-PnP tree.
155+
pub(crate) fn pnpm_pnp_layout(project_root: &Path) -> bool {
156+
let node_modules = project_root.join("node_modules");
157+
(node_modules.join(".modules.yaml").is_file() || node_modules.join(".pnpm").is_dir())
158+
&& project_root.join("pnpm-lock.yaml").is_file()
159+
&& !project_root.join("yarn.lock").is_file()
160+
}
161+
121162
#[cfg(test)]
122163
mod tests {
123164
use super::*;
@@ -382,9 +423,6 @@ mod tests {
382423
///
383424
/// Layout verified against a real `pnpm install` (pnpm 10.28.2).
384425
#[test]
385-
#[ignore = "RED: documents a real bug — detect_npm_pkg_manager misreports a \
386-
pnpm `node-linker=pnp` tree as yarn berry. The test is correct; \
387-
the detector fix was not part of this change."]
388426
fn pnpm_pnp_mode_is_pnpm_not_yarn_berry() {
389427
let d = tempfile::tempdir().unwrap();
390428
// Root markers emitted by `pnpm install` with node-linker=pnp.

‎crates/socket-patch-core/src/vendor/npm_flavor.rs‎

Lines changed: 80 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,11 @@
55
//! a `pnpm-lock.yaml` only routes to the pnpm backend when its
66
//! `lockfileVersion` is one we have fixtures for, and a `yarn.lock` routes
77
//! to classic or berry by its header (the v1 comment vs a top-level
8-
//! `__metadata:` key). Only yarn PnP projects (`.pnp.*` loaders) are
9-
//! refused outright — their packages never land on disk to stage.
8+
//! `__metadata:` key). Only PnP projects (`.pnp.*` loaders) are refused
9+
//! outright: yarn-berry PnP because its packages never land on disk to
10+
//! stage, and pnpm's own `node-linker=pnp` mode (same loader file, real
11+
//! package dirs) because the file: rewiring is unvalidated under that
12+
//! linker — each with its own code and remedy.
1013
//!
1114
//! The router fans `vendor`/`revert` out per detected flavor. All five
1215
//! flavors have real backends: package-lock ([`super::npm_lock`]),
@@ -83,7 +86,11 @@ const LOCKFILE_FAMILIES: [(NpmLockFlavor, &[&str]); 4] = [
8386
/// Probe the project root for the lockfile flavor that drives npm installs.
8487
///
8588
/// Decision table, first match wins:
86-
/// 1. a PnP loader file → Err `vendor_yarn_berry_unsupported`;
89+
/// 1. a PnP loader file → Err `vendor_yarn_berry_unsupported` — unless the
90+
/// tree is pnpm's own `node-linker=pnp` layout (pnpm-lock.yaml + installed
91+
/// pnpm store + no yarn.lock, see
92+
/// [`crate::crawlers::pkg_managers::pnpm_pnp_layout`]) → Err
93+
/// `vendor_pnpm_pnp_unsupported` with a pnpm remedy;
8794
/// 2. `bun.lock` → Bun; else `bun.lockb` → Err `vendor_bun_lockb_unsupported`;
8895
/// 3. `pnpm-lock.yaml` → head-sniff `lockfileVersion` (only `'9.0'`) → Pnpm,
8996
/// else Err `vendor_lockfile_version_unsupported`;
@@ -109,9 +116,28 @@ pub(crate) async fn detect_npm_lock_flavor(
109116
};
110117

111118
// 1. Yarn berry PnP — checked first because it means packages are not on
112-
// disk at all, whatever lockfiles are also lying around.
119+
// disk at all, whatever lockfiles are also lying around. Carve-out:
120+
// pnpm's own PnP mode (`node-linker=pnp` in `.npmrc`) writes the same
121+
// loader but is a pnpm project — recommending `yarn patch` there is
122+
// wrong twice over. Vendor still refuses (the file: rewiring has no
123+
// fixtures under pnpm's PnP linker — fail closed), but with a pnpm
124+
// diagnosis and remedy.
113125
for marker in PNP_MARKERS {
114126
if exists(marker).await {
127+
if crate::crawlers::pkg_managers::pnpm_pnp_layout(project_root) {
128+
return Err((
129+
"vendor_pnpm_pnp_unsupported",
130+
format!(
131+
"found `{marker}` alongside pnpm-lock.yaml and an installed pnpm \
132+
store: this is a pnpm project using `node-linker=pnp` (.npmrc), \
133+
not yarn berry — vendor's relative file: rewiring is not \
134+
validated under pnpm's Plug'n'Play linker; use `socket-patch \
135+
scan --mode hosted` (which edits pnpm-lock.yaml in place), or \
136+
switch .npmrc to `node-linker=isolated`, run `pnpm install`, \
137+
and re-run vendor"
138+
),
139+
));
140+
}
115141
return Err((
116142
"vendor_yarn_berry_unsupported",
117143
format!(
@@ -458,6 +484,56 @@ mod tests {
458484
}
459485
}
460486

487+
/// Stage the root markers a real `pnpm install` with
488+
/// `node-linker=pnp` emits (layout verified against pnpm 10.28.2):
489+
/// the `.pnp.cjs` loader, pnpm-lock.yaml, and an installed virtual
490+
/// store with the per-project pnpm marker.
491+
async fn stage_pnpm_pnp_layout(root: &Path) {
492+
touch(root, ".pnp.cjs", "/* pnp */").await;
493+
touch(root, "pnpm-lock.yaml", PNPM_9).await;
494+
tokio::fs::create_dir_all(
495+
root.join("node_modules/.pnpm/flatted@3.3.1/node_modules/flatted"),
496+
)
497+
.await
498+
.unwrap();
499+
touch(&root.join("node_modules"), ".modules.yaml", "").await;
500+
}
501+
502+
/// pnpm's own PnP mode (`node-linker=pnp`) writes a `.pnp.cjs` just
503+
/// like yarn-berry — the probe must refuse with a pnpm diagnosis and
504+
/// remedy, never `vendor_yarn_berry_unsupported` / "use yarn patch"
505+
/// (a yarn command in a pnpm repo).
506+
#[tokio::test]
507+
async fn pnpm_pnp_layout_refuses_with_pnpm_remedy() {
508+
let tmp = tempfile::tempdir().unwrap();
509+
stage_pnpm_pnp_layout(tmp.path()).await;
510+
let (code, detail) = detect_npm_lock_flavor(tmp.path()).await.unwrap_err();
511+
assert_eq!(code, "vendor_pnpm_pnp_unsupported");
512+
assert!(detail.contains("node-linker=pnp"), "{detail}");
513+
assert!(detail.contains("scan --mode hosted"), "{detail}");
514+
assert!(!detail.contains("yarn patch"), "{detail}");
515+
}
516+
517+
/// The pnpm-PnP carve-out stays fail-closed: a yarn.lock alongside
518+
/// the loader (ambiguous multi-PM tree), or a stale pnpm-lock.yaml
519+
/// with no installed pnpm store, keeps the yarn-berry refusal.
520+
#[tokio::test]
521+
async fn pnpm_pnp_carve_out_stays_yarn_berry_when_ambiguous() {
522+
// Both lockfiles present: ambiguous, yarn-berry refusal wins.
523+
let tmp = tempfile::tempdir().unwrap();
524+
stage_pnpm_pnp_layout(tmp.path()).await;
525+
touch(tmp.path(), "yarn.lock", YARN_BERRY).await;
526+
let (code, _) = detect_npm_lock_flavor(tmp.path()).await.unwrap_err();
527+
assert_eq!(code, "vendor_yarn_berry_unsupported");
528+
529+
// Stale pnpm-lock.yaml, no installed store: no escape either.
530+
let tmp = tempfile::tempdir().unwrap();
531+
touch(tmp.path(), ".pnp.cjs", "/* pnp */").await;
532+
touch(tmp.path(), "pnpm-lock.yaml", PNPM_9).await;
533+
let (code, _) = detect_npm_lock_flavor(tmp.path()).await.unwrap_err();
534+
assert_eq!(code, "vendor_yarn_berry_unsupported");
535+
}
536+
461537
#[tokio::test]
462538
async fn bun_lock_routes_and_lockb_refuses() {
463539
let tmp = tempfile::tempdir().unwrap();

0 commit comments

Comments
 (0)