Fix vendored pnpm package.json CRLF/BOM layout (#662)

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

@@ -197,7 +197,13 @@ fn new_ledgers_compact_whole_file_snapshots_and_revert() {
             assert_eq!(masked_tree(&f.root), pristine, "{eco}: byte-exact restore");
         } else {
             let reverted = read_tree(&fixtures_dir().join(eco).join("reverted"));
-            for (rel, bytes) in reverted {
+            for (rel, mut bytes) in reverted {
+                // The base binary always ended a re-rendered pnpm
+                // package.json with a newline; this binary keeps the
+                // file's own trailer, and the fixture's has none (#662).
+                if *eco == "pnpm" && rel == "package.json" {
+                    assert_eq!(bytes.pop(), Some(b'\n'), "{eco}: {rel}");
+                }
                 assert_eq!(
                     std::fs::read(f.root.join(&rel)).unwrap(),
                     bytes,

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

@@ -63,7 +63,7 @@ use crate::utils::fs::{
 };
 use crate::utils::socket_dir::remove_tree_and_prune;
 
-use super::common::{already_patched_result, detect_indent, done, refused, serialize_json};
+use super::common::{already_patched_result, done, parse_json_manifest, refused, JsonLayout};
 use super::npm_common::{
     done_failure_unstage, gate_packages, guard_coordinates, guard_revert_uuid_dir, refusal_code,
     stage_patch_pack, tgz_rel_leaf,
@@ -323,8 +323,9 @@ pub(super) async fn vendor_pnpm_dialect(
 
     // ── 6. Commit: package.json + pnpm-workspace.yaml FIRST, lock second,
     //    unwind the override surfaces on a lock failure (P3 desync safety).
-    let pkg_indent = detect_indent(&String::from_utf8_lossy(&pkg_bytes));
-    let new_pkg_bytes = match serialize_json(&pkg, &pkg_indent) {
+    // Re-render package.json in its own layout (BOM, indent, line ending,
+    // trailer) so a Windows / autocrlf manifest diffs only in the override.
+    let new_pkg_bytes = match JsonLayout::of(&String::from_utf8_lossy(&pkg_bytes)).render(&pkg) {
         Ok(bytes) => bytes,
         Err(e) => {
             return done_failure_unstage(
@@ -480,7 +481,7 @@ async fn read_project(
             )));
         }
     };
-    let pkg: Value = match serde_json::from_slice(&pkg_bytes) {
+    let pkg: Value = match parse_json_manifest(&pkg_bytes) {
         Ok(Value::Object(map)) => Value::Object(map),
         Ok(_) | Err(_) => {
             return Err(Box::new(refused(
@@ -845,13 +846,13 @@ pub(super) async fn revert_pnpm_dialect(
             Err(e) => return RevertOutcome::failed(format!("cannot read {PNPM_LOCK}: {e}")),
         }
     }
-    let mut pkg_state: Option<(Value, String)> = None; // (doc, indent)
+    let mut pkg_state: Option<(Value, JsonLayout)> = None;
     if touches_pkg {
         match read_regular_to_bytes(&project_root.join(PACKAGE_JSON)).await {
-            Ok(bytes) => match serde_json::from_slice::<Value>(&bytes) {
+            Ok(bytes) => match parse_json_manifest(&bytes) {
                 Ok(doc) if doc.is_object() => {
-                    let indent = detect_indent(&String::from_utf8_lossy(&bytes));
-                    pkg_state = Some((doc, indent));
+                    let layout = JsonLayout::of(&String::from_utf8_lossy(&bytes));
+                    pkg_state = Some((doc, layout));
                 }
                 // Fail-closed: editing a manifest we cannot parse risks
                 // destroying it; the user must repair it first.
@@ -944,8 +945,8 @@ pub(super) async fn revert_pnpm_dialect(
         }
     }
     if pkg_dirty {
-        if let Some((doc, indent)) = &pkg_state {
-            let bytes = match serialize_json(doc, indent) {
+        if let Some((doc, layout)) = &pkg_state {
+            let bytes = match layout.render(doc) {
                 Ok(b) => b,
                 Err(e) => {
                     return RevertOutcome::failed(format!("cannot serialize {PACKAGE_JSON}: {e}"))
@@ -5048,6 +5049,41 @@ snapshots:
             .exists());
     }
 
+    /// A CRLF, BOM or BOM+CRLF+tab `package.json` (a Windows / autocrlf
+    /// checkout) keeps its layout: the vendored file differs from the
+    /// original only in `pnpm.overrides`, and the revert is byte-exact
+    /// (#662).
+    #[tokio::test]
+    async fn vendor_and_revert_keep_package_json_layout() {
+        use crate::vendor::test_support::{relayout, JSON_LAYOUTS};
+        for (tag, bom, crlf, tab) in JSON_LAYOUTS {
+            let before = relayout(P1_BEFORE_PKG, bom, crlf, tab);
+            let fx = fixture_with(&before, P1_BEFORE_LOCK).await;
+            let (result, entry, _) = expect_done(fx.vendor(false).await);
+            assert!(result.success, "{tag}: {:?}", result.error);
+            assert_eq!(
+                fx.read(PACKAGE_JSON).await,
+                relayout(P1_AFTER_PKG, bom, crlf, tab),
+                "{tag}: vendored package.json keeps its layout"
+            );
+            assert_eq!(
+                fx.read(PNPM_LOCK).await,
+                P1_AFTER_LOCK.replace(SPIKE_INTEGRITY, &fx.actual_integrity().await),
+                "{tag}: lock unaffected by the manifest layout"
+            );
+
+            let outcome = revert_pnpm(&entry.unwrap(), fx.root(), false).await;
+            assert!(outcome.success, "{tag}: {:?}", outcome.error);
+            assert!(outcome.warnings.is_empty(), "{tag}: {:?}", outcome.warnings);
+            assert_eq!(
+                fx.read(PACKAGE_JSON).await,
+                before,
+                "{tag}: package.json byte-restored"
+            );
+            assert_eq!(fx.read(PNPM_LOCK).await, P1_BEFORE_LOCK, "{tag}");
+        }
+    }
+
     #[tokio::test]
     async fn revert_allowlist_is_fail_closed() {
         let fx = fixture_with(P1_BEFORE_PKG, P1_BEFORE_LOCK).await;

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

@@ -1992,6 +1992,45 @@ packages:
         }
     }
 
+    /// A CRLF, BOM or BOM+CRLF+tab `package.json` keeps its layout through
+    /// vendor and a byte-exact revert, for BOTH legacy grammars (#662).
+    #[tokio::test]
+    async fn vendor_and_revert_keep_package_json_layout() {
+        use crate::vendor::test_support::{relayout, JSON_LAYOUTS};
+        for (before_lock, after_lock, grammar) in [
+            (T7_BEFORE_LOCK, T7_AFTER_LOCK, "5.4"),
+            (T8_BEFORE_LOCK, T8_AFTER_LOCK, "6.0"),
+        ] {
+            for (layout, bom, crlf, tab) in JSON_LAYOUTS {
+                let tag = format!("{grammar} {layout}");
+                let before = relayout(T_BEFORE_PKG, bom, crlf, tab);
+                let fx = fixture_with(&before, before_lock).await;
+                let (result, entry, _) = expect_done(fx.vendor(false).await);
+                assert!(result.success, "{tag}: {:?}", result.error);
+                assert_eq!(
+                    fx.read(PACKAGE_JSON).await,
+                    relayout(T_AFTER_PKG, bom, crlf, tab),
+                    "{tag}: vendored package.json keeps its layout"
+                );
+                assert_eq!(
+                    fx.read(PNPM_LOCK).await,
+                    fx.expected_lock(after_lock).await,
+                    "{tag}: lock unaffected by the manifest layout"
+                );
+
+                let outcome = revert_pnpm_legacy(&entry.unwrap(), fx.root(), false).await;
+                assert!(outcome.success, "{tag}: {:?}", outcome.error);
+                assert!(outcome.warnings.is_empty(), "{tag}: {:?}", outcome.warnings);
+                assert_eq!(
+                    fx.read(PACKAGE_JSON).await,
+                    before,
+                    "{tag}: package.json byte-restored"
+                );
+                assert_eq!(fx.read(PNPM_LOCK).await, before_lock, "{tag}");
+            }
+        }
+    }
+
     #[tokio::test]
     async fn legacy_lifecycle_leaves_an_unreadable_workspace_file_untouched() {
         for lock in [T7_BEFORE_LOCK, T8_BEFORE_LOCK] {

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

@@ -992,3 +992,39 @@ pub(crate) fn expect_failed(
         }
     }
 }
+
+/// The `package.json` layouts a Windows / editor-written manifest arrives
+/// in, each paired with a tag for assertion messages: CRLF, a UTF-8 BOM,
+/// and BOM + CRLF + tab indent. `relayout(lf_text)` converts an LF,
+/// two-space-indented oracle into the same layout, so a vendored oracle
+/// and the pre-vendor bytes can both be compared byte-for-byte.
+pub(crate) const JSON_LAYOUTS: [(&str, bool, bool, bool); 3] = [
+    ("crlf", false, true, false),
+    ("bom", true, false, false),
+    ("bom+crlf+tab", true, true, true),
+];
+
+/// `text` (LF, two-space indent) in the given layout.
+pub(crate) fn relayout(text: &str, bom: bool, crlf: bool, tab: bool) -> String {
+    let mut out = String::new();
+    if bom {
+        out.push('\u{feff}');
+    }
+    for line in text.split_inclusive('\n') {
+        let line = if tab {
+            let body = line.trim_start_matches(' ');
+            let depth = (line.len() - body.len()) / 2;
+            format!("{}{body}", "\t".repeat(depth))
+        } else {
+            line.to_string()
+        };
+        match line.strip_suffix('\n') {
+            Some(content) if crlf => {
+                out.push_str(content);
+                out.push_str("\r\n");
+            }
+            _ => out.push_str(&line),
+        }
+    }
+    out
+}