Scaffold packages in keyless pnpm-workspace.yaml (#1096)

crates/socket-patch-cli/src/commands/scan/hosted.rs

@@ -2539,6 +2539,28 @@ mod tests {
         }
     }
 
+    /// #1096: an existing file with no keys at all (empty, comments only,
+    /// bare document markers) gains the root-only `packages` scaffold with
+    /// the trust key — pnpm 8.x–10.4 refuse a workspace file holding a key
+    /// but no `packages` — inside the document, every user byte intact.
+    #[test]
+    fn plan_workspace_trust_scaffolds_packages_in_a_keyless_file() {
+        const ADDED: &str = "packages:\n  - '.'\ntrustLockfile: true\n";
+        for (user, want) in [
+            ("", ADDED.to_string()),
+            ("\n", format!("{ADDED}\n")),
+            ("# settings\n", format!("# settings\n{ADDED}")),
+            ("---\n", format!("---\n{ADDED}")),
+            ("%YAML 1.2\n---\n", format!("%YAML 1.2\n---\n{ADDED}")),
+            ("---\n...\n", format!("---\n{ADDED}...\n")),
+        ] {
+            match plan_workspace_trust(Some(user)) {
+                TrustPlan::Scaffold(text) => assert_eq!(text, want, "{user:?}"),
+                _ => panic!("a keyless file must plan a Scaffold: {user:?}"),
+            }
+        }
+    }
+
     /// An existing workspace file gains exactly one line after its last
     /// non-empty line; every other byte — including a trailing blank line and
     /// comments — is preserved so a revert can remove exactly that line.

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

@@ -406,6 +406,127 @@ async fn hosted_trust_edit_reads_the_workspace_yaml_shape() {
     }
 }
 
+/// #1096: a pnpm-workspace.yaml with no keys at all (empty, comments only,
+/// bare document markers) is a "no workspace" file to pnpm, but pnpm
+/// 8.x–10.4 refuse every command once it holds a key without `packages`
+/// ("packages field missing or empty"). The trust edit therefore splices in
+/// the root-only `packages` scaffold with the trust key, and `rollback`
+/// takes both back out, restoring the user's file byte for byte. A project
+/// pinned to pnpm 9.0–10.4 gets no edit at all, as with no file (#734).
+#[tokio::test]
+#[serial]
+async fn hosted_trust_edit_scaffolds_packages_in_a_keyless_workspace_yaml() {
+    let server = MockServer::start().await;
+    mock_discovery(&server).await;
+    mock_reference(&server).await;
+
+    const ADDED: &str = "packages:\n  - '.'\ntrustLockfile: true\n";
+    for (user_ws, want) in [
+        (
+            "# pnpm settings go here\n",
+            format!("# pnpm settings go here\n{ADDED}"),
+        ),
+        ("\n", format!("{ADDED}\n")),
+        ("---\n", format!("---\n{ADDED}")),
+        ("%YAML 1.2\n---\n", format!("%YAML 1.2\n---\n{ADDED}")),
+        ("---\n...\n", format!("---\n{ADDED}...\n")),
+    ] {
+        let tmp = tempfile::tempdir().unwrap();
+        write_pnpm_project(tmp.path());
+        let ws_path = tmp.path().join("pnpm-workspace.yaml");
+        std::fs::write(&ws_path, user_ws).unwrap();
+
+        let code = run(hosted_args(tmp.path(), server.uri())).await;
+        assert_eq!(code, 0, "scan --mode hosted should succeed for {user_ws:?}");
+        assert_eq!(
+            std::fs::read_to_string(&ws_path).unwrap(),
+            want,
+            "a keyless workspace file gains the packages scaffold too ({user_ws:?})"
+        );
+
+        let code = rollback_hosted(tmp.path(), &server).await;
+        assert_eq!(
+            code, 0,
+            "rollback must restore the pnpm pin for {user_ws:?}"
+        );
+        assert_eq!(
+            std::fs::read_to_string(&ws_path).unwrap(),
+            user_ws,
+            "rollback restores the keyless workspace file byte for byte"
+        );
+    }
+
+    // An empty file reads as the created scaffold, which rollback deletes
+    // (pnpm reads an empty file and no file alike).
+    let tmp = tempfile::tempdir().unwrap();
+    write_pnpm_project(tmp.path());
+    let ws_path = tmp.path().join("pnpm-workspace.yaml");
+    std::fs::write(&ws_path, "").unwrap();
+    assert_eq!(run(hosted_args(tmp.path(), server.uri())).await, 0);
+    assert_eq!(std::fs::read_to_string(&ws_path).unwrap(), ADDED);
+    assert_eq!(rollback_hosted(tmp.path(), &server).await, 0);
+    assert!(
+        !ws_path.exists(),
+        "the scaffold-only file goes with the pin"
+    );
+
+    // A BOM-only file keeps its BOM first; several (empty) documents are
+    // not spliced into at all.
+    for (user_ws, want) in [
+        ("\u{feff}\n", format!("\u{feff}\n{ADDED}")),
+        ("---\n---\n", "---\n---\n".to_string()),
+    ] {
+        let tmp = tempfile::tempdir().unwrap();
+        write_pnpm_project(tmp.path());
+        let ws_path = tmp.path().join("pnpm-workspace.yaml");
+        std::fs::write(&ws_path, user_ws).unwrap();
+        assert_eq!(run(hosted_args(tmp.path(), server.uri())).await, 0);
+        assert_eq!(
+            std::fs::read_to_string(&ws_path).unwrap(),
+            want,
+            "{user_ws:?}"
+        );
+    }
+
+    // A lock installed from the keyless file as a multi-package workspace
+    // (pnpm <= 10.4 reads one as every nested package): a root-only
+    // scaffold would drop the members, so the file stays untouched.
+    let tmp = tempfile::tempdir().unwrap();
+    write_pnpm_project(tmp.path());
+    let lock_path = tmp.path().join("pnpm-lock.yaml");
+    let lock = std::fs::read_to_string(&lock_path).unwrap();
+    let with_member = lock.replacen("\npackages:", "\n  sub: {}\n\npackages:", 1);
+    assert_ne!(with_member, lock);
+    std::fs::write(&lock_path, with_member).unwrap();
+    let ws_path = tmp.path().join("pnpm-workspace.yaml");
+    std::fs::write(&ws_path, "# pnpm settings go here\n").unwrap();
+    assert_eq!(run(hosted_args(tmp.path(), server.uri())).await, 0);
+    assert_eq!(
+        std::fs::read_to_string(&ws_path).unwrap(),
+        "# pnpm settings go here\n",
+        "a keyless multi-package workspace is left alone"
+    );
+
+    // Pinned to pnpm 9.15.9: the keyless file stays untouched (#734).
+    let tmp = tempfile::tempdir().unwrap();
+    write_pnpm_project(tmp.path());
+    std::fs::write(
+        tmp.path().join("package.json"),
+        format!(
+            r#"{{ "name": "consumer", "version": "0.0.0", "packageManager": "pnpm@9.15.9", "dependencies": {{ "{NAME}": "{VERSION}" }} }}"#
+        ),
+    )
+    .unwrap();
+    let ws_path = tmp.path().join("pnpm-workspace.yaml");
+    std::fs::write(&ws_path, "# pnpm settings go here\n").unwrap();
+    assert_eq!(run(hosted_args(tmp.path(), server.uri())).await, 0);
+    assert_eq!(
+        std::fs::read_to_string(&ws_path).unwrap(),
+        "# pnpm settings go here\n",
+        "a pnpm 9 project's keyless workspace file is left alone"
+    );
+}
+
 /// #903 / #904: a `pnpm-lock.yaml` and `pnpm-workspace.yaml` saved with a
 /// UTF-8 BOM read like their plain twins. The BOM lock gets the
 /// `trustLockfile: true` auto-config (it used to read as unversioned and

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

@@ -13,7 +13,7 @@
 //!
 //! Pure text in, answers out; the editors own the reads and writes.
 
-use crate::formats::text::strip_bom;
+use crate::formats::text::{split_bom, strip_bom};
 
 /// The parsed key (quotes removed) and its inline value (comment and
 /// surrounding blanks stripped; `""` for a block-valued key) when `line` is
@@ -116,10 +116,65 @@ pub(crate) fn block_insert_point(lines: &[String]) -> Result<usize, String> {
     Ok(match (last, end_marker) {
         (Some(i), _) => i + 1,
         (None, Some(end)) => end,
-        (None, None) => lines.len(),
+        // Blank lines only: the new key opens the file, so its trailing
+        // newlines stay where they are — after a BOM line, though, which
+        // must stay the stream's first bytes.
+        (None, None) => usize::from(lines.first().is_some_and(|l| !split_bom(l).0.is_empty())),
     })
 }
 
+/// The root-only `packages:` list a splice writes into a workspace file
+/// that has none (pnpm 8.x–10.4 refuse a workspace file holding any key but
+/// no `packages`), in the vendor and hosted scaffold spelling.
+pub(crate) const PACKAGES_SCAFFOLD: [&str; 2] = ["packages:", "  - '.'"];
+
+/// Whether a pnpm-workspace.yaml holds no key at all: every line is blank,
+/// a comment, a `%` directive or a bare `---` / `...` marker. pnpm parses
+/// such a file to an empty document and installs as if it did not exist,
+/// but pnpm 8.x–10.4 refuse every command once it holds a key without a
+/// `packages` field ("packages field missing or empty", #1096), so a splice
+/// that gives it its first key must give it [`PACKAGES_SCAFFOLD`] too.
+pub(crate) fn is_keyless(text: &str) -> bool {
+    strip_bom(text).split('\n').all(|raw| {
+        let line = raw.strip_suffix('\r').unwrap_or(raw);
+        let trimmed = line.trim();
+        trimmed.is_empty()
+            || trimmed.starts_with('#')
+            || line.starts_with('%')
+            || ((is_marker(line, "---") || is_marker(line, "..."))
+                && strip_comment(&line[3..]).trim().is_empty())
+    })
+}
+
+/// Whether a pnpm lock's project document (`lines`) lists an importer
+/// besides the root `.`. A pnpm-workspace.yaml with no keys reads as every
+/// nested package on pnpm 8–10.4 (`packages` defaults to `['.', '**']`) but
+/// as the root alone on pnpm >= 10.5, so a lock made from one with member
+/// importers comes from pnpm <= 10.4, and the root-only
+/// [`PACKAGES_SCAFFOLD`] would drop those members.
+pub(crate) fn lock_has_member_importers(lines: &[String]) -> bool {
+    let Some((start, end)) = super::lines::section_bounds(lines, "importers") else {
+        return false;
+    };
+    lines[start + 1..end]
+        .iter()
+        .filter_map(|line| super::lines::parse_key_line(line.strip_suffix('\r').unwrap_or(line), 2))
+        .any(|(key, _, _)| key != ".")
+}
+
+/// `text` with the contiguous run of lines `block` removed, when that
+/// leaves a [`is_keyless`] document: the inverse of a splice that gave a
+/// keyless file its first keys. `None` when the run is absent or the rest
+/// of the file holds keys (the lines are then not provably the splice).
+pub(crate) fn strip_keyless_splice(text: &str, block: &[&str]) -> Option<String> {
+    let lines: Vec<&str> = text.split('\n').collect();
+    let at = lines.windows(block.len()).position(|w| w == block)?;
+    let mut rest = lines;
+    rest.drain(at..at + block.len());
+    let rest = rest.join("\n");
+    is_keyless(&rest).then_some(rest)
+}
+
 /// The top-level `name:` section holding a block mapping: `(header, end)`,
 /// `end` being the next column-0 line that is not a comment (exclusive) — a
 /// `#` comment at column 0 does not close a YAML block mapping. Unlike
@@ -421,6 +476,64 @@ mod tests {
         }
     }
 
+    #[test]
+    fn member_importers_mark_a_pre_10_5_workspace() {
+        let lock = |importers: &str| {
+            lines(&format!(
+                "lockfileVersion: '9.0'\n\nimporters:\n\n{importers}\npackages: {{}}\n"
+            ))
+        };
+        assert!(!lock_has_member_importers(&lock(
+            "  .:\n    dependencies: {}\n"
+        )));
+        assert!(lock_has_member_importers(&lock(
+            "  .:\n    dependencies: {}\n\n  sub:\n    dependencies: {}\n"
+        )));
+        assert!(!lock_has_member_importers(&lines(
+            "lockfileVersion: '9.0'\n"
+        )));
+    }
+
+    #[test]
+    fn keyless_documents_and_their_splices() {
+        for text in [
+            "",
+            "\n",
+            "# c\n\n  # indented\n",
+            "---\n",
+            "--- # start\n",
+            "%YAML 1.2\n---\n",
+            "---\n...\n",
+            "\u{feff}# bom\r\n",
+        ] {
+            assert!(is_keyless(text), "{text:?}");
+        }
+        for text in ["a: 1\n", "# c\npackages: []\n", "--- {}\n", "- a\n", "~\n"] {
+            assert!(!is_keyless(text), "{text:?}");
+        }
+        let block = ["packages:", "  - '.'", "trustLockfile: true"];
+        assert_eq!(
+            strip_keyless_splice("# c\npackages:\n  - '.'\ntrustLockfile: true\n", &block),
+            Some("# c\n".to_string())
+        );
+        assert_eq!(
+            strip_keyless_splice(
+                "---\npackages:\n  - '.'\ntrustLockfile: true\n...\n",
+                &block
+            ),
+            Some("---\n...\n".to_string())
+        );
+        assert_eq!(
+            strip_keyless_splice(
+                "packages:\n  - '.'\ntrustLockfile: true\ncatalog: {}\n",
+                &block
+            ),
+            None,
+            "other keys: not provably the splice"
+        );
+        assert_eq!(strip_keyless_splice("# c\n", &block), None);
+    }
+
     #[test]
     fn block_insert_point_follows_the_document() {
         assert_eq!(block_insert_point(&lines("a: 1\nb:\n  - c\n")), Ok(3));
@@ -429,7 +542,10 @@ mod tests {
         assert_eq!(block_insert_point(&lines("a: 1\n\n...\n# done\n")), Ok(1));
         assert_eq!(block_insert_point(&lines("%YAML 1.2\n---\na: 1\n")), Ok(3));
         assert_eq!(block_insert_point(&lines("# only a comment\n")), Ok(1));
-        assert_eq!(block_insert_point(&lines("")), Ok(1));
+        assert_eq!(block_insert_point(&lines("")), Ok(0));
+        assert_eq!(block_insert_point(&lines("\n\n")), Ok(0));
+        assert_eq!(block_insert_point(&lines("\u{feff}\n")), Ok(1));
+        assert_eq!(block_insert_point(&lines("\u{feff}")), Ok(1));
         for text in [
             "{a: 1}\n",
             "[a]\n",