Fix vendored revert leaving created scaffold behind (#636, #670)

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

@@ -1297,6 +1297,134 @@ fn pnpm8_lock_v60_hermetic_splice_idempotency_and_revert() {
     run_legacy_hermetic(PNPM8_LOCK, PNPM8_AFTER_TEMPLATE, "6.0");
 }
 
+/// #636: two packages vendored into a project with no `pnpm` table and no
+/// pnpm-workspace.yaml, then `vendor --revert` (purl order: is-number, the
+/// entry that created both, goes first). The lock was already byte-exact;
+/// package.json kept `"pnpm": { "overrides": {} }` and the scaffolded
+/// workspace file stayed behind. Both must now come back byte-identical.
+#[test]
+fn pnpm9_two_packages_vendor_revert_removes_created_scaffold() {
+    const LOCK: &str = "lockfileVersion: '9.0'
+
+settings:
+  autoInstallPeers: true
+  excludeLinksFromLockfile: false
+
+importers:
+
+  .:
+    dependencies:
+      is-number:
+        specifier: 7.0.0
+        version: 7.0.0
+      left-pad:
+        specifier: 1.3.0
+        version: 1.3.0
+
+packages:
+
+  is-number@7.0.0:
+    resolution: {integrity: sha512-41Cifkg6e8TylSpdtTpeLVMqvSBEVzTttHvERD741+pnZ8ANv0004MRL43QKPDlK9cGvNp6NZWZUBlbGXYxxng==}
+    engines: {node: '>=0.12.0'}
+
+  left-pad@1.3.0:
+    resolution: {integrity: sha512-XI5MPzVNApjAyhQzphX8BkmKsKUxD4LdyK24iZeQGinBN9yTQT3bFlCBy/aVx2HrNcqQGsdot8ghrjyrvMCoEA==}
+    deprecated: use String.prototype.padStart()
+
+snapshots:
+
+  is-number@7.0.0: {}
+
+  left-pad@1.3.0: {}
+";
+    let tmp = tempfile::tempdir().unwrap();
+    let proj = tmp.path().join("proj");
+    let orig = b"module.exports = function (x) { return x; };\n".to_vec();
+    let patched: Vec<u8> = [MARKER.as_bytes(), orig.as_slice()].concat();
+    let mut patches = serde_json::Map::new();
+    for (name, version, uuid) in [
+        ("left-pad", "1.3.0", UUID),
+        ("is-number", "7.0.0", "2b3c4d5e-6f70-4a1b-8c2d-0123456789ac"),
+    ] {
+        let dir = proj.join("node_modules").join(name);
+        std::fs::create_dir_all(&dir).unwrap();
+        std::fs::write(
+            dir.join("package.json"),
+            format!("{{\"name\":\"{name}\",\"version\":\"{version}\"}}\n"),
+        )
+        .unwrap();
+        std::fs::write(dir.join("index.js"), &orig).unwrap();
+        patches.insert(
+            format!("pkg:npm/{name}@{version}"),
+            serde_json::json!({
+                "uuid": uuid,
+                "exportedAt": "2026-01-01T00:00:00Z",
+                "files": { "package/index.js": {
+                    "beforeHash": git_sha256(&orig),
+                    "afterHash": git_sha256(&patched),
+                }},
+                "vulnerabilities": {},
+                "description": "two-package scaffold patch",
+                "license": "MIT",
+                "tier": "free",
+            }),
+        );
+    }
+    let socket = proj.join(".socket");
+    std::fs::create_dir_all(socket.join("blobs")).unwrap();
+    std::fs::write(
+        socket.join("manifest.json"),
+        serde_json::to_string_pretty(&serde_json::json!({ "patches": patches })).unwrap(),
+    )
+    .unwrap();
+    std::fs::write(socket.join("blobs").join(git_sha256(&patched)), &patched).unwrap();
+
+    let pkg_before = "{\n  \"name\": \"fx\",\n  \"version\": \"1.0.0\",\n  \"private\": true,\n  \
+                      \"dependencies\": {\n    \"left-pad\": \"1.3.0\",\n    \
+                      \"is-number\": \"7.0.0\"\n  }\n}\n";
+    std::fs::write(proj.join("package.json"), pkg_before).unwrap();
+    std::fs::write(proj.join("pnpm-lock.yaml"), LOCK).unwrap();
+    let cwd = proj.to_str().unwrap();
+
+    let (code, stdout, stderr) =
+        run_socket(&proj, &["vendor", "--json", "--offline", "--cwd", cwd]);
+    assert_eq!(code, 0, "vendor.\nstdout:\n{stdout}\nstderr:\n{stderr}");
+    let env = parse_envelope(&stdout);
+    assert_eq!(
+        env["summary"]["applied"], 2,
+        "both packages vendored: {env}"
+    );
+    assert!(
+        proj.join("pnpm-workspace.yaml").is_file(),
+        "a 9.0 lock mirrors overrides into a scaffolded workspace file"
+    );
+
+    let (code, stdout, stderr) = run_socket(
+        &proj,
+        &["vendor", "--revert", "--json", "--offline", "--cwd", cwd],
+    );
+    assert_eq!(code, 0, "revert.\nstdout:\n{stdout}\nstderr:\n{stderr}");
+    let renv = parse_envelope(&stdout);
+    assert_eq!(
+        renv["summary"]["removed"], 2,
+        "both entries reverted: {renv}"
+    );
+    assert_eq!(
+        std::fs::read_to_string(proj.join("package.json")).unwrap(),
+        pkg_before,
+        "no empty pnpm.overrides left behind"
+    );
+    assert_eq!(
+        std::fs::read_to_string(proj.join("pnpm-lock.yaml")).unwrap(),
+        LOCK
+    );
+    assert!(
+        !proj.join("pnpm-workspace.yaml").exists(),
+        "the scaffolded pnpm-workspace.yaml must be deleted"
+    );
+    assert!(!proj.join(".socket/vendor").exists());
+}
+
 /// The tarball's SRI (`sha512-<base64>`), the integrity spelling pnpm locks
 /// record.
 fn tarball_integrity(tgz: &Path) -> String {

crates/socket-patch-cli/tests/fixtures/legacy-ledgers/pnpm/reverted/package.json

@@ -5,8 +5,5 @@
   "dependencies": {
     "alpha": "1.0.0",
     "beta": "1.0.0"
-  },
-  "pnpm": {
-    "overrides": {}
   }
 }

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

@@ -113,12 +113,22 @@ fn group_commit_ends_where_per_package_commits_end_for_every_ecosystem() {
             masked_tree(&oracle.root),
             "{eco}: same tree after --revert"
         );
-        // Two ecosystems keep scaffolding their revert does not remove when
-        // TWO packages were vendored (the emptied pnpm override tables and
-        // workspace file; the catch-all `<packageSourceMapping>` nuget adds
-        // to a config that had none). That predates the group commit — the
-        // oracle leaves the same bytes, asserted just above — so only the
-        // others are held to a byte-exact round trip here.
+        // Two ecosystems do not round-trip byte for byte when TWO packages
+        // were vendored: pnpm rewrites the fixture's minified package.json
+        // with an indent, and nuget keeps the catch-all
+        // `<packageSourceMapping>` it adds to a config that had none. That
+        // predates the group commit — the oracle leaves the same bytes,
+        // asserted just above — so only the others are held to a
+        // byte-exact round trip here. pnpm's created scaffold itself is
+        // gone (#636): no emptied `pnpm.overrides`, no workspace file.
+        if *eco == "pnpm" {
+            assert!(
+                !grouped.root.join("pnpm-workspace.yaml").exists(),
+                "pnpm: the scaffolded workspace file is removed"
+            );
+            let pkg = std::fs::read_to_string(grouped.root.join("package.json")).unwrap();
+            assert!(!pkg.contains("\"pnpm\""), "pnpm: {pkg}");
+        }
         if !["pnpm", "nuget"].contains(eco) {
             let after = masked_tree(&grouped.root);
             assert!(

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

@@ -8860,4 +8860,196 @@ snapshots:
             }
         }
     }
+
+    // ── two packages sharing created scaffolding (#636) ──────────────────
+
+    const TWO_PKG: &str = r#"{
+  "name": "fx",
+  "version": "1.0.0",
+  "private": true,
+  "dependencies": {
+    "left-pad": "1.3.0",
+    "is-number": "7.0.0"
+  }
+}
+"#;
+
+    const TWO_LOCK: &str = "lockfileVersion: '9.0'
+
+settings:
+  autoInstallPeers: true
+  excludeLinksFromLockfile: false
+
+importers:
+
+  .:
+    dependencies:
+      is-number:
+        specifier: 7.0.0
+        version: 7.0.0
+      left-pad:
+        specifier: 1.3.0
+        version: 1.3.0
+
+packages:
+
+  is-number@7.0.0:
+    resolution: {integrity: sha512-41Cifkg6e8TylSpdtTpeLVMqvSBEVzTttHvERD741+pnZ8ANv0004MRL43QKPDlK9cGvNp6NZWZUBlbGXYxxng==}
+    engines: {node: '>=0.12.0'}
+
+  left-pad@1.3.0:
+    resolution: {integrity: sha512-XI5MPzVNApjAyhQzphX8BkmKsKUxD4LdyK24iZeQGinBN9yTQT3bFlCBy/aVx2HrNcqQGsdot8ghrjyrvMCoEA==}
+    deprecated: use String.prototype.padStart()
+
+snapshots:
+
+  is-number@7.0.0: {}
+
+  left-pad@1.3.0: {}
+";
+
+    const IS_NUMBER_UUID: &str = "4d5e6f70-8a9b-4c0d-9e1f-2a3b4c5d6e7f";
+    const IS_NUMBER: &str = "pkg:npm/is-number@7.0.0";
+    const LEFT_PAD: &str = "pkg:npm/left-pad@1.3.0";
+
+    /// Vendor is-number then left-pad (is-number creates every scaffold),
+    /// persisting the ledger after each the way the vendor loop does.
+    async fn vendor_two(pkg_json: &str, workspace: Option<&str>) -> Fixture {
+        let fx = fixture_with(pkg_json, TWO_LOCK).await;
+        if let Some(ws) = workspace {
+            tokio::fs::write(fx.root().join(PNPM_WORKSPACE), ws)
+                .await
+                .unwrap();
+        }
+        let is_number = fx.root().join("node_modules/is-number");
+        tokio::fs::create_dir_all(&is_number).await.unwrap();
+        tokio::fs::write(
+            is_number.join("package.json"),
+            br#"{"name":"is-number","version":"7.0.0"}"#,
+        )
+        .await
+        .unwrap();
+        tokio::fs::write(is_number.join("index.js"), ORIG_INDEX)
+            .await
+            .unwrap();
+        let mut is_number_record = fx.record.clone();
+        is_number_record.uuid = IS_NUMBER_UUID.to_string();
+
+        let blobs = fx.root().join(".socket/blobs");
+        let sources = PatchSources::blobs_only(&blobs);
+        let mut state = crate::vendor::state::VendorState::new();
+        for (purl, dir, record) in [
+            (IS_NUMBER, is_number.clone(), &is_number_record),
+            (LEFT_PAD, fx.installed(), &fx.record),
+        ] {
+            let outcome = crate::vendor::test_support::vendor_pnpm(
+                purl,
+                &dir,
+                fx.root(),
+                record,
+                &sources,
+                "2026-06-09T00:00:00Z",
+                false,
+                false,
+                None,
+            )
+            .await;
+            let (result, entry, _) = expect_done(outcome);
+            assert!(result.success, "{purl}: {:?}", result.error);
+            state.entries.insert(purl.to_string(), entry.unwrap());
+            crate::vendor::state::save_state(fx.root(), &state)
+                .await
+                .unwrap();
+        }
+        fx
+    }
+
+    /// Revert `order` one entry at a time from a freshly loaded ledger,
+    /// saving after each removal (`vendor --revert`, `rollback` and
+    /// successive `remove` runs all persist per entry).
+    async fn revert_in_order(fx: &Fixture, order: [&str; 2]) {
+        for key in order {
+            let mut state = crate::vendor::state::load_state(fx.root()).await.unwrap();
+            let entry = state.entries.get(key).cloned().unwrap();
+            let outcome = revert_pnpm(&entry, fx.root(), false).await;
+            assert!(outcome.success, "{key}: {:?}", outcome.error);
+            assert!(outcome.warnings.is_empty(), "{key}: {:?}", outcome.warnings);
+            state.entries.remove(key);
+            crate::vendor::state::save_state(fx.root(), &state)
+                .await
+                .unwrap();
+        }
+    }
+
+    /// #636: the creator (is-number, first in purl order) reverted first
+    /// must not leave `"pnpm": { "overrides": {} }` or the scaffolded
+    /// pnpm-workspace.yaml behind once left-pad empties them.
+    #[tokio::test]
+    async fn revert_two_packages_creator_first_removes_created_scaffold() {
+        let fx = vendor_two(TWO_PKG, None).await;
+        assert!(fx.root().join(PNPM_WORKSPACE).exists());
+
+        revert_in_order(&fx, [IS_NUMBER, LEFT_PAD]).await;
+        assert_eq!(fx.read(PACKAGE_JSON).await, TWO_PKG);
+        assert_eq!(fx.read(PNPM_LOCK).await, TWO_LOCK);
+        assert!(
+            !fx.root().join(PNPM_WORKSPACE).exists(),
+            "scaffolded workspace file left behind"
+        );
+    }
+
+    /// #636, `remove` in the other order: clean before the fix too.
+    #[tokio::test]
+    async fn revert_two_packages_creator_last_removes_created_scaffold() {
+        let fx = vendor_two(TWO_PKG, None).await;
+
+        revert_in_order(&fx, [LEFT_PAD, IS_NUMBER]).await;
+        assert_eq!(fx.read(PACKAGE_JSON).await, TWO_PKG);
+        assert!(!fx.root().join(PNPM_WORKSPACE).exists());
+    }
+
+    /// The `overrides:` section vendoring added to the user's own
+    /// pnpm-workspace.yaml goes too, in creator-first order; the file and
+    /// the user's keys stay.
+    #[tokio::test]
+    async fn revert_two_packages_removes_created_workspace_overrides() {
+        let ws = "packages:\n  - '.'\n";
+        let fx = vendor_two(TWO_PKG, Some(ws)).await;
+
+        revert_in_order(&fx, [IS_NUMBER, LEFT_PAD]).await;
+        assert_eq!(fx.read(PNPM_WORKSPACE).await, ws);
+        assert_eq!(fx.read(PACKAGE_JSON).await, TWO_PKG);
+    }
+
+    /// A ledger written before the flags were shared (only the creator
+    /// flagged) is repaired on load, so it unwinds cleanly too.
+    #[tokio::test]
+    async fn revert_two_packages_repairs_a_creator_only_ledger() {
+        let fx = vendor_two(TWO_PKG, None).await;
+        let path = fx.root().join(".socket/vendor/state.json");
+        let mut ledger: serde_json::Value =
+            serde_json::from_slice(&tokio::fs::read(&path).await.unwrap()).unwrap();
+        ledger["entries"][LEFT_PAD]["pnpm"] = serde_json::json!({});
+        tokio::fs::write(&path, serde_json::to_vec_pretty(&ledger).unwrap())
+            .await
+            .unwrap();
+
+        revert_in_order(&fx, [IS_NUMBER, LEFT_PAD]).await;
+        assert_eq!(fx.read(PACKAGE_JSON).await, TWO_PKG);
+        assert!(!fx.root().join(PNPM_WORKSPACE).exists());
+    }
+
+    /// A user's own `pnpm` table keeps its keys; only the overrides table
+    /// vendoring created goes.
+    #[tokio::test]
+    async fn revert_two_packages_keeps_a_user_pnpm_table() {
+        let pkg = TWO_PKG.replace(
+            "  }\n}\n",
+            "  },\n  \"pnpm\": {\n    \"onlyBuiltDependencies\": []\n  }\n}\n",
+        );
+        let fx = vendor_two(&pkg, None).await;
+
+        revert_in_order(&fx, [IS_NUMBER, LEFT_PAD]).await;
+        assert_eq!(fx.read(PACKAGE_JSON).await, pkg);
+    }
 }