Fix vendored pnpm shadowing workspace overrides (#360)

crates/socket-patch-cli/tests/e2e_vendor_pnpm_build.rs

@@ -1881,3 +1881,122 @@ fn pnpm_agent_apply_patches_a_transitive_dep_in_a_relocated_virtual_store() {
         assert_eq!(std::fs::read(&index).unwrap(), orig, "{setting}");
     }
 }
+
+/// #360: on pnpm 10.5+ a project may keep its `overrides:` in
+/// pnpm-workspace.yaml, and a package.json `pnpm.overrides` then REPLACES
+/// them. Vendoring must wire only the workspace file and the lock, so the
+/// committable set still frozen-installs (no
+/// ERR_PNPM_LOCKFILE_CONFIG_MISMATCH) with the user's override intact and
+/// the patched bytes installed, and `vendor --revert` restores every file.
+#[test]
+fn pnpm_vendor_keeps_user_workspace_overrides_authoritative() {
+    if !has_corepack_pm(PNPM_PRIMARY) {
+        println!("SKIP: `corepack {PNPM_PRIMARY}` unavailable");
+        return;
+    }
+    let pm = PNPM_PRIMARY;
+    let tmp = tempfile::tempdir().unwrap();
+    let proj = tmp.path().join("proj");
+    std::fs::create_dir_all(&proj).unwrap();
+    let pkg_before = format!(
+        "{{\"name\":\"ws-overrides\",\"version\":\"0.0.0\",\"private\":true,\
+         \"dependencies\":{{\"{DEP}\":\"{DEP_VERSION}\",\"is-odd\":\"3.0.1\"}}}}\n"
+    );
+    let ws_before = "packages:\n  - '.'\noverrides:\n  is-number: 7.0.0\n";
+    std::fs::write(proj.join("package.json"), &pkg_before).unwrap();
+    std::fs::write(proj.join("pnpm-workspace.yaml"), ws_before).unwrap();
+
+    let store = tmp.path().join("pnpm-store");
+    let install = corepack(
+        &proj,
+        pm,
+        &["install", "--store-dir", store.to_str().unwrap()],
+    );
+    if !install.status.success() {
+        assert!(!pnpm_required(), "fixture install failed: {install:?}");
+        println!("SKIP: fixture `pnpm install` failed: {install:?}");
+        return;
+    }
+    let lock_path = proj.join("pnpm-lock.yaml");
+    let lock_before = std::fs::read_to_string(&lock_path).unwrap();
+    if !lock_before.contains("overrides:\n  is-number: 7.0.0\n") {
+        // pnpm 10.0-10.4 ignore workspace-file overrides; nothing to prove.
+        println!("SKIP: {pm} does not read overrides from pnpm-workspace.yaml");
+        return;
+    }
+
+    let index = proj.join("node_modules").join(DEP).join("index.js");
+    let orig = std::fs::read(&index).unwrap();
+    let patched: Vec<u8> = [MARKER.as_bytes(), orig.as_slice()].concat();
+    let purl = format!("pkg:npm/{DEP}@{DEP_VERSION}");
+    stage_patch(&proj, &purl, "package/index.js", &orig, &patched);
+    let cwd = proj.to_str().unwrap();
+
+    let (code, stdout, stderr) =
+        run_socket(&proj, &["vendor", "--json", "--offline", "--cwd", cwd]);
+    assert_eq!(code, 0, "vendor failed.\n{stdout}\n{stderr}");
+    let env = parse_envelope(&stdout);
+    assert_eq!(env["summary"]["applied"], 1, "{env}");
+    assert_eq!(
+        std::fs::read_to_string(proj.join("package.json")).unwrap(),
+        pkg_before,
+        "package.json must not gain a pnpm.overrides that shadows the workspace overrides"
+    );
+    let ws_after = std::fs::read_to_string(proj.join("pnpm-workspace.yaml")).unwrap();
+    assert!(
+        ws_after.starts_with(ws_before)
+            && ws_after.contains(&format!("{DEP}@{DEP_VERSION}: file:")),
+        "{ws_after}"
+    );
+
+    // Fresh checkout of the committable files, empty store.
+    let fresh = tmp.path().join("fresh");
+    std::fs::create_dir_all(&fresh).unwrap();
+    for file in ["package.json", "pnpm-lock.yaml", "pnpm-workspace.yaml"] {
+        std::fs::copy(proj.join(file), fresh.join(file)).unwrap();
+    }
+    copy_dir_recursive(&proj.join(".socket"), &fresh.join(".socket"));
+    let fresh_store = tmp.path().join("fresh-pnpm-store");
+    let ci = corepack(
+        &fresh,
+        pm,
+        &[
+            "install",
+            "--frozen-lockfile",
+            "--store-dir",
+            fresh_store.to_str().unwrap(),
+        ],
+    );
+    assert!(
+        ci.status.success(),
+        "fresh `pnpm install --frozen-lockfile` must accept the vendored wiring.\nstdout:\n{}\nstderr:\n{}",
+        String::from_utf8_lossy(&ci.stdout),
+        String::from_utf8_lossy(&ci.stderr),
+    );
+    assert_eq!(
+        std::fs::read(fresh.join("node_modules").join(DEP).join("index.js")).unwrap(),
+        patched,
+        "the patched bytes are installed"
+    );
+    // The user's override still applies: is-odd's is-number is 7.0.0.
+    let script = "const p=require('path');process.stdout.write(require(require.resolve(\
+         'is-number/package.json',{paths:[p.dirname(require.resolve('is-odd'))]})).version)";
+    let out = Command::new("node")
+        .args(["-e", script])
+        .current_dir(&fresh)
+        .output()
+        .expect("node runs");
+    assert_eq!(String::from_utf8_lossy(&out.stdout), "7.0.0", "{out:?}");
+
+    let (code, stdout, stderr) = run_socket(&proj, &["vendor", "--revert", "--json", "--cwd", cwd]);
+    assert_eq!(code, 0, "revert failed.\n{stdout}\n{stderr}");
+    assert_eq!(
+        std::fs::read_to_string(proj.join("package.json")).unwrap(),
+        pkg_before
+    );
+    assert_eq!(
+        std::fs::read_to_string(proj.join("pnpm-workspace.yaml")).unwrap(),
+        ws_before
+    );
+    assert_eq!(std::fs::read_to_string(&lock_path).unwrap(), lock_before);
+}

crates/socket-patch-core/src/vendor/pnpm_lock.rs

@@ -255,7 +255,16 @@ pub(super) async fn vendor_pnpm_dialect(
     };
     let mut wiring: Vec<WiringRecord> = Vec::new();
 
-    let (pkg_changed, created_pnpm_table, created_overrides_table) =
+    // The package.json copy is a back-compat surface for pnpm that reads
+    // overrides only from package.json. When the lock shows the project's
+    // pnpm reads them from pnpm-workspace.yaml, a new package.json
+    // `pnpm.overrides` would REPLACE the user's workspace overrides on
+    // pnpm 10 (#360), so only the workspace file and the lock are wired.
+    let skip_pkg_copy = dialect == PnpmDialect::V9
+        && workspace_overrides_govern(&pkg, ws_text.as_deref(), lock.lines());
+    let (pkg_changed, created_pnpm_table, created_overrides_table) = if skip_pkg_copy {
+        (false, false, false)
+    } else {
         match apply_pkg_override(&mut pkg, &effective_key, &spec, &mut wiring) {
             Ok(out) => out,
             Err(e) => {
@@ -268,7 +277,8 @@ pub(super) async fn vendor_pnpm_dialect(
                 )
                 .await
             }
-        };
+        }
+    };
     let (lock_changed, lock_warning) = match lock.edit(&ctx, &mut wiring) {
         Ok(edit) => edit,
         Err(e) => {
@@ -1699,6 +1709,38 @@ pub(super) fn apply_pkg_override(
     Ok((true, created_pnpm_table, created_overrides_table))
 }
 
+/// Does the pnpm that wrote this lock read `overrides:` from
+/// pnpm-workspace.yaml (pnpm 10.5+) rather than package.json? True when
+/// package.json has no `pnpm.overrides` of its own and the lock's
+/// `overrides:` records a user-authored (non-vendored) key of the
+/// workspace file's `overrides:` section. pnpm 9 and 10.0–10.4 ignore the
+/// workspace block, so their locks never record it; and with no user
+/// workspace override both surfaces agree anyway, so the package.json
+/// copy stays harmless there.
+fn workspace_overrides_govern(pkg: &Value, ws_text: Option<&str>, lock: &[String]) -> bool {
+    if pkg.get("pnpm").and_then(|p| p.get("overrides")).is_some() {
+        return false;
+    }
+    let Some(text) = ws_text else {
+        return false;
+    };
+    let ws_lines = split_lines(text);
+    let Some((ws_start, ws_end, indent)) = ws_overrides_section(&ws_lines) else {
+        return false;
+    };
+    let Some((lock_start, lock_end)) = section_bounds(lock, "overrides") else {
+        return false;
+    };
+    let lock_keys: Vec<&str> = lock[lock_start + 1..lock_end]
+        .iter()
+        .filter_map(|l| parse_key_line(l, 2).map(|(key, _, _)| key))
+        .collect();
+    ws_lines[ws_start + 1..ws_end]
+        .iter()
+        .filter_map(|l| parse_key_line(l, indent))
+        .any(|(key, _, rest)| !is_vendor_value(rest) && lock_keys.contains(&key))
+}
+
 // ─────────────────────── pnpm-workspace.yaml override ─────────────────────
 // pnpm >= 11 reads `overrides:` only from pnpm-workspace.yaml, so the same
 // `<name>@<version>` → `file:` mapping is mirrored here. Edits are line
@@ -5534,6 +5576,118 @@ snapshots:
         );
     }
 
+    /// [`P1_BEFORE_LOCK`] as pnpm 10.5+ writes it when the user's
+    /// `is-number: 6.0.0` override lives in pnpm-workspace.yaml: the lock's
+    /// `overrides:` records the workspace-file override.
+    fn p1_lock_with_ws_user_override() -> String {
+        P1_BEFORE_LOCK.replacen(
+            "\nimporters:\n",
+            "\noverrides:\n  is-number: 6.0.0\n\nimporters:\n",
+            1,
+        )
+    }
+    const WS_WITH_USER_OVERRIDE: &str = "packages:\n  - '.'\noverrides:\n  is-number: 6.0.0\n";
+
+    /// #360: when the lock shows pnpm reads `overrides:` from
+    /// pnpm-workspace.yaml (pnpm 10.5+), a new package.json
+    /// `pnpm.overrides` would REPLACE the user's workspace overrides on
+    /// pnpm 10 (frozen installs fail ERR_PNPM_LOCKFILE_CONFIG_MISMATCH, a
+    /// re-lock drops them). Vendor wires the workspace file and the lock
+    /// only, leaving package.json byte-identical; revert restores both.
+    #[tokio::test]
+    async fn workspace_read_overrides_skip_the_package_json_copy() {
+        let lock = p1_lock_with_ws_user_override();
+        let fx = fixture_with(P1_BEFORE_PKG, &lock).await;
+        write_ws(&fx, WS_WITH_USER_OVERRIDE).await;
+
+        let (_, entry, _) = expect_done(fx.vendor(false).await);
+        let entry = entry.unwrap();
+        let spec = format!("file:{}", fx.rel_tgz());
+        assert_eq!(
+            fx.read(PACKAGE_JSON).await,
+            P1_BEFORE_PKG,
+            "package.json must not gain a pnpm.overrides that shadows the workspace overrides"
+        );
+        assert!(
+            entry.wiring.iter().all(|r| r.kind != KIND_PKG_OVERRIDE),
+            "no package.json wiring is recorded"
+        );
+        assert_eq!(
+            fx.read(PNPM_WORKSPACE).await,
+            format!("{WS_WITH_USER_OVERRIDE}  left-pad@1.3.0: {spec}\n"),
+        );
+        let new_lock = fx.read(PNPM_LOCK).await;
+        assert!(
+            new_lock.contains(&format!(
+                "overrides:\n  is-number: 6.0.0\n  left-pad@1.3.0: {spec}\n"
+            )),
+            "the lock's override map equals the workspace file's: {new_lock}"
+        );
+
+        // A re-run is in sync and still leaves package.json alone.
+        let (result, again, _) = expect_done(fx.vendor(false).await);
+        assert!(result.success, "{:?}", result.error);
+        assert!(again.is_none(), "in-sync re-run records nothing");
+        assert!(result
+            .files_verified
+            .iter()
+            .all(|v| v.status == crate::patch::apply::VerifyStatus::AlreadyPatched));
+        assert_eq!(fx.read(PACKAGE_JSON).await, P1_BEFORE_PKG);
+
+        let outcome = revert_pnpm(&entry, fx.root(), false).await;
+        assert!(outcome.success, "{:?}", outcome.error);
+        assert_eq!(fx.read(PACKAGE_JSON).await, P1_BEFORE_PKG);
+        assert_eq!(fx.read(PNPM_WORKSPACE).await, WS_WITH_USER_OVERRIDE);
+        assert_eq!(
+            fx.read(PNPM_LOCK).await,
+            lock,
+            "lock restored byte-for-byte"
+        );
+    }
+
+    /// Control for #360: workspace overrides the lock does NOT record were
+    /// ignored by the pnpm that wrote it (pnpm 9, 10.0–10.4 read only
+    /// package.json), so the package.json copy is still written there.
+    #[tokio::test]
+    async fn workspace_overrides_unrecorded_in_lock_keep_the_package_json_copy() {
+        let fx = fixture_with(P1_BEFORE_PKG, P1_BEFORE_LOCK).await;
+        write_ws(&fx, WS_WITH_USER_OVERRIDE).await;
+
+        let (_, entry, _) = expect_done(fx.vendor(false).await);
+        assert_eq!(fx.read(PACKAGE_JSON).await, P1_AFTER_PKG);
+        assert!(entry
+            .unwrap()
+            .wiring
+            .iter()
+            .any(|r| r.kind == KIND_PKG_OVERRIDE));
+    }
+
+    /// Control for #360: a project that already keeps a package.json
+    /// `pnpm.overrides` (which pnpm 10 reads in place of the workspace
+    /// block) keeps getting the package.json copy.
+    #[tokio::test]
+    async fn existing_package_json_overrides_keep_the_package_json_copy() {
+        let pkg = P1_BEFORE_PKG.replacen(
+            "\n  }\n}\n",
+            "\n  },\n  \"pnpm\": {\n    \"overrides\": {\n      \"is-number\": \"6.0.0\"\n    }\n  }\n}\n",
+            1,
+        );
+        let fx = fixture_with(&pkg, &p1_lock_with_ws_user_override()).await;
+        write_ws(&fx, WS_WITH_USER_OVERRIDE).await;
+
+        let (_, entry, _) = expect_done(fx.vendor(false).await);
+        let pkg_after: Value = serde_json::from_str(&fx.read(PACKAGE_JSON).await).unwrap();
+        assert_eq!(
+            pkg_after["pnpm"]["overrides"]["left-pad@1.3.0"],
+            Value::String(format!("file:{}", fx.rel_tgz()))
+        );
+        assert!(entry
+            .unwrap()
+            .wiring
+            .iter()
+            .any(|r| r.kind == KIND_PKG_OVERRIDE));
+    }
+
     /// A flow-style/inline `overrides:` mapping the line surgery cannot
     /// splice into is refused before any write.
     #[tokio::test]