Fix pnpm 7/8 vendored specifier YAML quoting (#754)

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

@@ -1086,8 +1086,8 @@ async fn pnpm_pinned_matrix_vendored_lifecycle_and_manifestless_vex() {
             run_pnpm_capstone(&pm, VendorDriver::VendorCli).await;
             run_pnpm_capstone(&pm, VendorDriver::GetUuid).await;
         }
-        7 => off_runtime(|| run_legacy_capstone(&pm, "lockfileVersion: 5.4")),
-        8 => off_runtime(|| run_legacy_capstone(&pm, "lockfileVersion: '6.0'")),
+        7 => off_runtime(|| run_legacy_capstone(&pm, "lockfileVersion: 5.4", "proj")),
+        8 => off_runtime(|| run_legacy_capstone(&pm, "lockfileVersion: '6.0'", "proj")),
         _ => off_runtime(|| run_unsupported_lock_refusal(&pm)),
     }
 }
@@ -1488,7 +1488,7 @@ fn pnpm7_real_lifecycle_same_path_frozen_and_moved_checkout_offline() {
         println!("SKIP: `corepack {PNPM_LEGACY_7}` unavailable");
         return;
     }
-    run_legacy_capstone(PNPM_LEGACY_7, "lockfileVersion: 5.4");
+    run_legacy_capstone(PNPM_LEGACY_7, "lockfileVersion: 5.4", "proj");
 }
 
 #[test]
@@ -1497,7 +1497,40 @@ fn pnpm8_real_lifecycle_same_path_frozen_and_moved_checkout_offline() {
         println!("SKIP: `corepack {PNPM_LEGACY_8}` unavailable");
         return;
     }
-    run_legacy_capstone(PNPM_LEGACY_8, "lockfileVersion: '6.0'");
+    run_legacy_capstone(PNPM_LEGACY_8, "lockfileVersion: '6.0'", "proj");
+}
+
+/// Project dir names holding a YAML indicator (#754). Windows forbids `:`
+/// in a path component, so the `: ` case runs on unix only.
+#[cfg(not(windows))]
+const YAML_INDICATOR_DIRS: &[&str] = &["hash #x", "colon: x"];
+#[cfg(windows)]
+const YAML_INDICATOR_DIRS: &[&str] = &["hash #x"];
+
+/// #754: the absolute specifier lands in the lock under a project path
+/// holding YAML indicators. Unquoted, ` #` turned the rest of the path
+/// into a comment (ERR_PNPM_OUTDATED_LOCKFILE) and `: ` broke the line
+/// (ERR_PNPM_BROKEN_LOCKFILE); the same-path frozen install must pass.
+#[test]
+fn pnpm7_real_lifecycle_under_yaml_indicator_paths() {
+    if !has_corepack_pm(PNPM_LEGACY_7) {
+        println!("SKIP: `corepack {PNPM_LEGACY_7}` unavailable");
+        return;
+    }
+    for dir in YAML_INDICATOR_DIRS {
+        run_legacy_capstone(PNPM_LEGACY_7, "lockfileVersion: 5.4", dir);
+    }
+}
+
+#[test]
+fn pnpm8_real_lifecycle_under_yaml_indicator_paths() {
+    if !has_corepack_pm(PNPM_LEGACY_8) {
+        println!("SKIP: `corepack {PNPM_LEGACY_8}` unavailable");
+        return;
+    }
+    for dir in YAML_INDICATOR_DIRS {
+        run_legacy_capstone(PNPM_LEGACY_8, "lockfileVersion: '6.0'", dir);
+    }
 }
 
 /// Full lifecycle against the REAL pinned legacy pnpm, spike-proven flags:
@@ -1515,9 +1548,9 @@ fn pnpm8_real_lifecycle_same_path_frozen_and_moved_checkout_offline() {
 ///    marker bytes (probe C);
 /// 5. idempotent re-vendor (byte-stable, already_vendored);
 /// 6. revert restores both files byte-identical and removes .socket/vendor.
-fn run_legacy_capstone(pm: &str, lock_head: &str) {
+fn run_legacy_capstone(pm: &str, lock_head: &str, proj_dir: &str) {
     let tmp = tempfile::tempdir().unwrap();
-    let proj = tmp.path().join("proj");
+    let proj = tmp.path().join(proj_dir);
     std::fs::create_dir_all(&proj).unwrap();
     let pkg_doc = serde_json::json!({
         "name": "pnpm-legacy-capstone",

crates/socket-patch-core/src/formats/pnpm/lines.rs

@@ -129,6 +129,89 @@ pub(crate) fn unquote_value(value: &str) -> &str {
     }
 }
 
+/// Spell a string as a block-mapping VALUE the way pnpm's YAML writer
+/// (js-yaml `dump`) does: plain when the plain scalar reads back as the
+/// same string, single-quoted otherwise, double-quoted (escaped) when it
+/// holds a character single quotes can't carry. A value we splice in
+/// verbatim must not contain ` #` (the rest becomes a comment) or `: `
+/// (the line stops being valid YAML) unquoted — e.g. an absolute path
+/// under a directory named `My Project #2`.
+pub(crate) fn yaml_value(value: &str) -> std::borrow::Cow<'_, str> {
+    use std::borrow::Cow;
+    if !value.chars().all(is_yaml_printable) {
+        let mut out = String::with_capacity(value.len() + 2);
+        out.push('"');
+        for c in value.chars() {
+            match c {
+                '"' => out.push_str("\\\""),
+                '\\' => out.push_str("\\\\"),
+                '\n' => out.push_str("\\n"),
+                '\t' => out.push_str("\\t"),
+                '\r' => out.push_str("\\r"),
+                c if is_yaml_printable(c) => out.push(c),
+                c if (c as u32) <= 0xFF => out.push_str(&format!("\\x{:02X}", c as u32)),
+                c if (c as u32) <= 0xFFFF => out.push_str(&format!("\\u{:04X}", c as u32)),
+                c => out.push_str(&format!("\\U{:08X}", c as u32)),
+            }
+        }
+        out.push('"');
+        return Cow::Owned(out);
+    }
+    if is_plain_safe(value) {
+        Cow::Borrowed(value)
+    } else {
+        Cow::Owned(format!("'{}'", value.replace('\'', "''")))
+    }
+}
+
+/// js-yaml's `isPrintable`: what a single-quoted or plain scalar may hold.
+fn is_yaml_printable(c: char) -> bool {
+    let c = c as u32;
+    (0x20..=0x7E).contains(&c)
+        || ((0xA1..=0xD7FF).contains(&c) && c != 0x2028 && c != 0x2029)
+        || ((0xE000..=0xFFFD).contains(&c) && c != 0xFEFF)
+        || (0x10000..=0x10FFFF).contains(&c)
+}
+
+/// Would `value` (all printable) read back unchanged as a plain block
+/// scalar? Mirrors js-yaml's plain-style rules for block context: no
+/// indicator first character, no leading/trailing space or trailing `:`,
+/// no `#` after a space, no `:` before a space, and nothing YAML would
+/// resolve to a non-string (null, bool, number).
+fn is_plain_safe(value: &str) -> bool {
+    let Some(first) = value.chars().next() else {
+        return false;
+    };
+    if first == ' '
+        || "-?:,[]{}#&*!|>'\"%@`".contains(first)
+        || value.ends_with(' ')
+        || value.ends_with(':')
+        || value.contains(" #")
+        || value.contains(": ")
+    {
+        return false;
+    }
+    let lower = value.to_ascii_lowercase();
+    let implicit = matches!(
+        lower.as_str(),
+        "~" | "null"
+            | "true"
+            | "false"
+            | "yes"
+            | "no"
+            | "on"
+            | "off"
+            | "y"
+            | "n"
+            | ".inf"
+            | "-.inf"
+            | "+.inf"
+            | ".nan"
+    ) || value.parse::<f64>().is_ok()
+        || (value.starts_with("0x") || value.starts_with("0o") || value.starts_with("0b"));
+    !implicit
+}
+
 /// pnpm quotes `@`-leading keys with single quotes; everything we write is
 /// otherwise bare.
 pub(crate) fn yaml_key(key: &str) -> String {
@@ -147,3 +230,61 @@ pub(crate) fn yaml_key_like(key: &str, original_repr: &str) -> String {
         _ => yaml_key(key),
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    /// Values pnpm's writer leaves plain stay byte-identical — the spellings
+    /// the vendored legacy splice already round-trips (#754's passing cells).
+    #[test]
+    fn yaml_value_keeps_plain_safe_values_plain() {
+        for v in [
+            "file:/tmp/w/plain dir/.socket/vendor/npm/u/left-pad-1.3.0.tgz",
+            "file:/tmp/w/ünïcode/x.tgz",
+            "file:/tmp/w/a'quote/x.tgz",
+            "file:/tmp/w/[br]/x.tgz",
+            "file:/tmp/w/x#y/x.tgz",
+            "file:C:/Users/x/a:b/x.tgz",
+            "^1.3.0",
+            "1.3.0",
+            "npm:left-pad@1.2.0",
+        ] {
+            assert_eq!(yaml_value(v), v, "{v}");
+        }
+    }
+
+    /// ` #` and `: ` would truncate or break a plain scalar, so they are
+    /// single-quoted exactly as pnpm 7/8 write them (#754).
+    #[test]
+    fn yaml_value_single_quotes_comment_and_mapping_indicators() {
+        assert_eq!(
+            yaml_value("file:/tmp/w/hash #x/a.tgz"),
+            "'file:/tmp/w/hash #x/a.tgz'"
+        );
+        assert_eq!(
+            yaml_value("file:/tmp/w/colon: x/a.tgz"),
+            "'file:/tmp/w/colon: x/a.tgz'"
+        );
+        assert_eq!(
+            yaml_value("file:/tmp/My Project #2/it's.tgz"),
+            "'file:/tmp/My Project #2/it''s.tgz'"
+        );
+        assert_eq!(yaml_value("trailing:"), "'trailing:'");
+        assert_eq!(yaml_value(" lead"), "' lead'");
+        assert_eq!(yaml_value("@scope/x"), "'@scope/x'");
+        assert_eq!(yaml_value("catalog:"), "'catalog:'");
+        assert_eq!(yaml_value("true"), "'true'");
+        assert_eq!(yaml_value("1.0"), "'1.0'");
+        assert_eq!(yaml_value(""), "''");
+    }
+
+    /// Characters single quotes can't carry fall back to an escaped
+    /// double-quoted scalar.
+    #[test]
+    fn yaml_value_double_quotes_non_printables() {
+        assert_eq!(yaml_value("a\tb"), "\"a\\tb\"");
+        assert_eq!(yaml_value("a\nb\"c\\"), "\"a\\nb\\\"c\\\\\"");
+        assert_eq!(yaml_value("a\u{7f}"), "\"a\\x7F\"");
+    }
+}