Skip to content

Commit c17f305

Browse files
mikolalysenkoclaude
andcommitted
Scaffold packages in keyless pnpm-workspace.yaml
A pnpm-workspace.yaml that is empty or holds only comments or document markers reads as "no workspace" to pnpm. Hosted scans appended only `trustLockfile: true` and vendored scans only an `overrides:` block to such a file, and pnpm 8.x-10.4 then refused every command with "packages field missing or empty", even though the scan reported success. Both writers now splice the root-only `packages: ['.']` scaffold in with their key when the file has no keys, and follow the no-file #734 rule: a project pinned to pnpm 9.0-10.4 gets no workspace edit. Hosted rollback and vendored revert take the spliced lines back out, so the user's file comes back byte for byte (an empty file reads as the created scaffold and is deleted by hosted rollback, as before). Fixes #1096 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
1 parent da5c575 commit c17f305

11 files changed

Lines changed: 375 additions & 27 deletions

File tree

‎crates/socket-patch-cli/CLI_CONTRACT.md‎

Lines changed: 4 additions & 4 deletions
Large diffs are not rendered by default.

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

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2539,6 +2539,28 @@ mod tests {
25392539
}
25402540
}
25412541

2542+
/// #1096: an existing file with no keys at all (empty, comments only,
2543+
/// bare document markers) gains the root-only `packages` scaffold with
2544+
/// the trust key — pnpm 8.x–10.4 refuse a workspace file holding a key
2545+
/// but no `packages` — inside the document, every user byte intact.
2546+
#[test]
2547+
fn plan_workspace_trust_scaffolds_packages_in_a_keyless_file() {
2548+
const ADDED: &str = "packages:\n - '.'\ntrustLockfile: true\n";
2549+
for (user, want) in [
2550+
("", ADDED.to_string()),
2551+
("\n", format!("{ADDED}\n")),
2552+
("# settings\n", format!("# settings\n{ADDED}")),
2553+
("---\n", format!("---\n{ADDED}")),
2554+
("%YAML 1.2\n---\n", format!("%YAML 1.2\n---\n{ADDED}")),
2555+
("---\n...\n", format!("---\n{ADDED}...\n")),
2556+
] {
2557+
match plan_workspace_trust(Some(user)) {
2558+
TrustPlan::Scaffold(text) => assert_eq!(text, want, "{user:?}"),
2559+
_ => panic!("a keyless file must plan a Scaffold: {user:?}"),
2560+
}
2561+
}
2562+
}
2563+
25422564
/// An existing workspace file gains exactly one line after its last
25432565
/// non-empty line; every other byte — including a trailing blank line and
25442566
/// comments — is preserved so a revert can remove exactly that line.

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

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -406,6 +406,90 @@ async fn hosted_trust_edit_reads_the_workspace_yaml_shape() {
406406
}
407407
}
408408

409+
/// #1096: a pnpm-workspace.yaml with no keys at all (empty, comments only,
410+
/// bare document markers) is a "no workspace" file to pnpm, but pnpm
411+
/// 8.x–10.4 refuse every command once it holds a key without `packages`
412+
/// ("packages field missing or empty"). The trust edit therefore splices in
413+
/// the root-only `packages` scaffold with the trust key, and `rollback`
414+
/// takes both back out, restoring the user's file byte for byte. A project
415+
/// pinned to pnpm 9.0–10.4 gets no edit at all, as with no file (#734).
416+
#[tokio::test]
417+
#[serial]
418+
async fn hosted_trust_edit_scaffolds_packages_in_a_keyless_workspace_yaml() {
419+
let server = MockServer::start().await;
420+
mock_discovery(&server).await;
421+
mock_reference(&server).await;
422+
423+
const ADDED: &str = "packages:\n - '.'\ntrustLockfile: true\n";
424+
for (user_ws, want) in [
425+
(
426+
"# pnpm settings go here\n",
427+
format!("# pnpm settings go here\n{ADDED}"),
428+
),
429+
("\n", format!("{ADDED}\n")),
430+
("---\n", format!("---\n{ADDED}")),
431+
("%YAML 1.2\n---\n", format!("%YAML 1.2\n---\n{ADDED}")),
432+
("---\n...\n", format!("---\n{ADDED}...\n")),
433+
] {
434+
let tmp = tempfile::tempdir().unwrap();
435+
write_pnpm_project(tmp.path());
436+
let ws_path = tmp.path().join("pnpm-workspace.yaml");
437+
std::fs::write(&ws_path, user_ws).unwrap();
438+
439+
let code = run(hosted_args(tmp.path(), server.uri())).await;
440+
assert_eq!(code, 0, "scan --mode hosted should succeed for {user_ws:?}");
441+
assert_eq!(
442+
std::fs::read_to_string(&ws_path).unwrap(),
443+
want,
444+
"a keyless workspace file gains the packages scaffold too ({user_ws:?})"
445+
);
446+
447+
let code = rollback_hosted(tmp.path(), &server).await;
448+
assert_eq!(
449+
code, 0,
450+
"rollback must restore the pnpm pin for {user_ws:?}"
451+
);
452+
assert_eq!(
453+
std::fs::read_to_string(&ws_path).unwrap(),
454+
user_ws,
455+
"rollback restores the keyless workspace file byte for byte"
456+
);
457+
}
458+
459+
// An empty file reads as the created scaffold, which rollback deletes
460+
// (pnpm reads an empty file and no file alike).
461+
let tmp = tempfile::tempdir().unwrap();
462+
write_pnpm_project(tmp.path());
463+
let ws_path = tmp.path().join("pnpm-workspace.yaml");
464+
std::fs::write(&ws_path, "").unwrap();
465+
assert_eq!(run(hosted_args(tmp.path(), server.uri())).await, 0);
466+
assert_eq!(std::fs::read_to_string(&ws_path).unwrap(), ADDED);
467+
assert_eq!(rollback_hosted(tmp.path(), &server).await, 0);
468+
assert!(
469+
!ws_path.exists(),
470+
"the scaffold-only file goes with the pin"
471+
);
472+
473+
// Pinned to pnpm 9.15.9: the keyless file stays untouched (#734).
474+
let tmp = tempfile::tempdir().unwrap();
475+
write_pnpm_project(tmp.path());
476+
std::fs::write(
477+
tmp.path().join("package.json"),
478+
format!(
479+
r#"{{ "name": "consumer", "version": "0.0.0", "packageManager": "pnpm@9.15.9", "dependencies": {{ "{NAME}": "{VERSION}" }} }}"#
480+
),
481+
)
482+
.unwrap();
483+
let ws_path = tmp.path().join("pnpm-workspace.yaml");
484+
std::fs::write(&ws_path, "# pnpm settings go here\n").unwrap();
485+
assert_eq!(run(hosted_args(tmp.path(), server.uri())).await, 0);
486+
assert_eq!(
487+
std::fs::read_to_string(&ws_path).unwrap(),
488+
"# pnpm settings go here\n",
489+
"a pnpm 9 project's keyless workspace file is left alone"
490+
);
491+
}
492+
409493
/// #903 / #904: a `pnpm-lock.yaml` and `pnpm-workspace.yaml` saved with a
410494
/// UTF-8 BOM read like their plain twins. The BOM lock gets the
411495
/// `trustLockfile: true` auto-config (it used to read as unversioned and

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

Lines changed: 81 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -116,10 +116,48 @@ pub(crate) fn block_insert_point(lines: &[String]) -> Result<usize, String> {
116116
Ok(match (last, end_marker) {
117117
(Some(i), _) => i + 1,
118118
(None, Some(end)) => end,
119-
(None, None) => lines.len(),
119+
// Blank lines only: the new key opens the file, so its trailing
120+
// newlines stay where they are.
121+
(None, None) => 0,
120122
})
121123
}
122124

125+
/// The root-only `packages:` list a splice writes into a workspace file
126+
/// that has none (pnpm 8.x–10.4 refuse a workspace file holding any key but
127+
/// no `packages`), in the vendor and hosted scaffold spelling.
128+
pub(crate) const PACKAGES_SCAFFOLD: [&str; 2] = ["packages:", " - '.'"];
129+
130+
/// Whether a pnpm-workspace.yaml holds no key at all: every line is blank,
131+
/// a comment, a `%` directive or a bare `---` / `...` marker. pnpm parses
132+
/// such a file to an empty document and installs as if it did not exist,
133+
/// but pnpm 8.x–10.4 refuse every command once it holds a key without a
134+
/// `packages` field ("packages field missing or empty", #1096), so a splice
135+
/// that gives it its first key must give it [`PACKAGES_SCAFFOLD`] too.
136+
pub(crate) fn is_keyless(text: &str) -> bool {
137+
strip_bom(text).split('\n').all(|raw| {
138+
let line = raw.strip_suffix('\r').unwrap_or(raw);
139+
let trimmed = line.trim();
140+
trimmed.is_empty()
141+
|| trimmed.starts_with('#')
142+
|| line.starts_with('%')
143+
|| ((is_marker(line, "---") || is_marker(line, "..."))
144+
&& strip_comment(&line[3..]).trim().is_empty())
145+
})
146+
}
147+
148+
/// `text` with the contiguous run of lines `block` removed, when that
149+
/// leaves a [`is_keyless`] document: the inverse of a splice that gave a
150+
/// keyless file its first keys. `None` when the run is absent or the rest
151+
/// of the file holds keys (the lines are then not provably the splice).
152+
pub(crate) fn strip_keyless_splice(text: &str, block: &[&str]) -> Option<String> {
153+
let lines: Vec<&str> = text.split('\n').collect();
154+
let at = lines.windows(block.len()).position(|w| w == block)?;
155+
let mut rest = lines;
156+
rest.drain(at..at + block.len());
157+
let rest = rest.join("\n");
158+
is_keyless(&rest).then_some(rest)
159+
}
160+
123161
/// The top-level `name:` section holding a block mapping: `(header, end)`,
124162
/// `end` being the next column-0 line that is not a comment (exclusive) — a
125163
/// `#` comment at column 0 does not close a YAML block mapping. Unlike
@@ -421,6 +459,46 @@ mod tests {
421459
}
422460
}
423461

462+
#[test]
463+
fn keyless_documents_and_their_splices() {
464+
for text in [
465+
"",
466+
"\n",
467+
"# c\n\n # indented\n",
468+
"---\n",
469+
"--- # start\n",
470+
"%YAML 1.2\n---\n",
471+
"---\n...\n",
472+
"\u{feff}# bom\r\n",
473+
] {
474+
assert!(is_keyless(text), "{text:?}");
475+
}
476+
for text in ["a: 1\n", "# c\npackages: []\n", "--- {}\n", "- a\n", "~\n"] {
477+
assert!(!is_keyless(text), "{text:?}");
478+
}
479+
let block = ["packages:", " - '.'", "trustLockfile: true"];
480+
assert_eq!(
481+
strip_keyless_splice("# c\npackages:\n - '.'\ntrustLockfile: true\n", &block),
482+
Some("# c\n".to_string())
483+
);
484+
assert_eq!(
485+
strip_keyless_splice(
486+
"---\npackages:\n - '.'\ntrustLockfile: true\n...\n",
487+
&block
488+
),
489+
Some("---\n...\n".to_string())
490+
);
491+
assert_eq!(
492+
strip_keyless_splice(
493+
"packages:\n - '.'\ntrustLockfile: true\ncatalog: {}\n",
494+
&block
495+
),
496+
None,
497+
"other keys: not provably the splice"
498+
);
499+
assert_eq!(strip_keyless_splice("# c\n", &block), None);
500+
}
501+
424502
#[test]
425503
fn block_insert_point_follows_the_document() {
426504
assert_eq!(block_insert_point(&lines("a: 1\nb:\n - c\n")), Ok(3));
@@ -429,7 +507,8 @@ mod tests {
429507
assert_eq!(block_insert_point(&lines("a: 1\n\n...\n# done\n")), Ok(1));
430508
assert_eq!(block_insert_point(&lines("%YAML 1.2\n---\na: 1\n")), Ok(3));
431509
assert_eq!(block_insert_point(&lines("# only a comment\n")), Ok(1));
432-
assert_eq!(block_insert_point(&lines("")), Ok(1));
510+
assert_eq!(block_insert_point(&lines("")), Ok(0));
511+
assert_eq!(block_insert_point(&lines("\n\n")), Ok(0));
433512
for text in [
434513
"{a: 1}\n",
435514
"[a]\n",

‎crates/socket-patch-core/src/hosted/engine.rs‎

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -49,10 +49,11 @@ use super::guidance::{
4949
plan_workspace_trust, pnpm_heal_root, pnpm_is_shrinkwrap_lock, pnpm_lock_may_need_store_flag,
5050
pnpm_lock_version_major, pnpm_root_only_workspace_breaks_add, pnpm_trust_configured_detail,
5151
pnpm_trust_legacy_detail, pnpm_trust_manual_guidance, pnpm_trust_not_needed_detail,
52-
pnpm_trust_policy_preamble, pnpm_trust_rush_detail, pnpm_trust_workspace_unreadable_detail,
53-
pnpm_trust_workspace_unsupported_detail, read_npmrc_for_allow_remote, read_workspace_for_trust,
54-
url_host, TrustPlan, NPM_LOCKS, NPM_REPLACE_REGISTRY_HOST_CODE, PNPM_TRUST_RUSH_MIXED_NOTE,
55-
PNPM_TRUST_TRADEOFF_AND_CAUTION, PNPM_WORKSPACE_REL, REDIRECT_PNPM_WORKSPACE_TRUST_EDIT_KIND,
52+
pnpm_trust_policy_preamble, pnpm_trust_rush_detail, pnpm_trust_scaffolded_detail,
53+
pnpm_trust_workspace_unreadable_detail, pnpm_trust_workspace_unsupported_detail,
54+
read_npmrc_for_allow_remote, read_workspace_for_trust, url_host, TrustPlan, NPM_LOCKS,
55+
NPM_REPLACE_REGISTRY_HOST_CODE, PNPM_TRUST_RUSH_MIXED_NOTE, PNPM_TRUST_TRADEOFF_AND_CAUTION,
56+
PNPM_WORKSPACE_REL, REDIRECT_PNPM_WORKSPACE_TRUST_EDIT_KIND,
5657
};
5758
use super::vlt::bun_lockb_present;
5859

@@ -2116,15 +2117,23 @@ fn pnpm_trust(
21162117
original: None,
21172118
new: Some(serde_json::json!("true")),
21182119
};
2119-
// No workspace file and a project pinned to pnpm 9.0–10.4: creating
2120-
// one would make it a root-only workspace those releases refuse
2121-
// `pnpm add` in, for a key they never read (#734).
2120+
// No workspace file (or one with no keys, which pnpm reads the
2121+
// same) and a project pinned to pnpm 9.0–10.4: creating one would
2122+
// make it a root-only workspace those releases refuse `pnpm add`
2123+
// in, for a key they never read (#734, #1096).
21222124
let pinned_pre_10_5 = match &workspace {
2123-
Ok(None) if !symlinked => root_only_workspace_breaks_add(view),
2125+
Ok(text)
2126+
if !symlinked
2127+
&& text
2128+
.as_deref()
2129+
.is_none_or(crate::formats::pnpm::workspace::is_keyless) =>
2130+
{
2131+
root_only_workspace_breaks_add(view)
2132+
}
21242133
_ => None,
21252134
};
21262135
match workspace {
2127-
Ok(None) if pinned_pre_10_5.is_some() => pnpm_trust_not_needed_detail(
2136+
Ok(_) if pinned_pre_10_5.is_some() => pnpm_trust_not_needed_detail(
21282137
&server,
21292138
pinned_pre_10_5.as_deref().unwrap_or_default(),
21302139
options.dry_run,
@@ -2142,6 +2151,10 @@ fn pnpm_trust(
21422151
trust_config_write = Some((text, trust_edit("added")));
21432152
pnpm_trust_configured_detail(&server, false, options.dry_run)
21442153
}
2154+
TrustPlan::Scaffold(text) => {
2155+
trust_config_write = Some((text, trust_edit("added")));
2156+
pnpm_trust_scaffolded_detail(&server, options.dry_run)
2157+
}
21452158
TrustPlan::AlreadyTrue => {
21462159
pnpm_rerun_only = spliced_pnpm_locks == 0;
21472160
let detail = pnpm_trust_already_true_detail(&server, PNPM_WORKSPACE_REL);

‎crates/socket-patch-core/src/hosted/guidance.rs‎

Lines changed: 47 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -257,15 +257,39 @@ pub fn pnpm_trust_configured_detail(server: &str, created: bool, dry_run: bool)
257257
(true, true) => "`trustLockfile: true` would be written to a new",
258258
(false, true) => "`trustLockfile: true` would be merged into the existing",
259259
};
260+
let undo = format!("delete {PNPM_WORKSPACE_REL}");
261+
trust_configured_detail(server, how, created.then_some(undo.as_str()), dry_run)
262+
}
263+
264+
/// The auto-config variant for an existing pnpm-workspace.yaml with no keys
265+
/// ([`TrustPlan::Scaffold`]): the trust key went in with the root-only
266+
/// `packages` field, which makes the project a root-only workspace just as
267+
/// a created file does.
268+
pub fn pnpm_trust_scaffolded_detail(server: &str, dry_run: bool) -> String {
269+
let how = if dry_run {
270+
"`trustLockfile: true` (with the root-only `packages: ['.']` that pnpm \
271+
8–10.4 require once the file holds a key) would be merged into the existing"
272+
} else {
273+
"`trustLockfile: true` (with the root-only `packages: ['.']` that pnpm \
274+
8–10.4 require once the file holds a key) was merged into the existing"
275+
};
276+
let undo =
277+
format!("remove the added `packages` and `trustLockfile` lines from {PNPM_WORKSPACE_REL}");
278+
trust_configured_detail(server, how, Some(&undo), dry_run)
279+
}
280+
281+
/// `undo` (set when the edit made the project a root-only workspace) names
282+
/// how to take the edit back.
283+
fn trust_configured_detail(server: &str, how: &str, undo: Option<&str>, dry_run: bool) -> String {
260284
// A created file makes the project a root-only workspace, where pnpm
261285
// 9.0–10.4 refuse `pnpm add` without `-w` (#734); a project pinned to
262286
// those releases never gets one, so the note names the pin as the way
263287
// out for an unpinned project with no install record.
264-
let root_only = if created {
288+
let root_only = if let Some(undo) = undo {
265289
let then = if dry_run {
266290
"before the real run".to_string()
267291
} else {
268-
format!("then delete {PNPM_WORKSPACE_REL} and re-run")
292+
format!("then {undo} and re-run")
269293
};
270294
format!(
271295
" On pnpm 9.0–10.4 a root-only workspace needs `pnpm add -w <pkg>` \
@@ -318,6 +342,13 @@ pub enum TrustPlan {
318342
/// Workspace file exists without a `trustLockfile:` key: append exactly
319343
/// one line after the last non-empty line, every other byte preserved.
320344
Append(String),
345+
/// Workspace file exists but holds no key at all (empty, comments only,
346+
/// bare document markers — "no workspace" to pnpm): the root-only
347+
/// `packages` scaffold goes in with the trust key, since pnpm 8.x–10.4
348+
/// refuse a workspace file holding a key but no `packages` (#1096).
349+
/// Every user byte is preserved, so rollback can take both back out
350+
/// ([`TRUST_SCAFFOLD_LINES`]).
351+
Scaffold(String),
321352
/// Already `trustLockfile: true` — nothing to write.
322353
AlreadyTrue,
323354
/// The user explicitly set `trustLockfile: <value>` (non-true). Their
@@ -330,6 +361,15 @@ pub enum TrustPlan {
330361
Unsupported(String),
331362
}
332363

364+
/// The lines the trust auto-config writes into a file with no keys (and,
365+
/// newline-terminated, the whole file it creates): the root-only `packages`
366+
/// scaffold and the trust key.
367+
pub const TRUST_SCAFFOLD_LINES: [&str; 3] = [
368+
crate::formats::pnpm::workspace::PACKAGES_SCAFFOLD[0],
369+
crate::formats::pnpm::workspace::PACKAGES_SCAFFOLD[1],
370+
"trustLockfile: true",
371+
];
372+
333373
/// Decide how to ensure `trustLockfile: true` in pnpm-workspace.yaml.
334374
/// Line splices only (never a YAML library), mirroring the vendor backend's
335375
/// workspace surgery: untouched lines stay byte-identical, so a revert can
@@ -340,6 +380,11 @@ pub fn plan_workspace_trust(existing: Option<&str>) -> TrustPlan {
340380
return TrustPlan::Create("packages:\n - '.'\ntrustLockfile: true\n".to_string());
341381
};
342382
let mut lines: Vec<String> = text.split('\n').map(str::to_string).collect();
383+
if crate::formats::pnpm::workspace::is_keyless(text) {
384+
let anchor = block_insert_point(&lines).unwrap_or(lines.len());
385+
lines.splice(anchor..anchor, TRUST_SCAFFOLD_LINES.map(str::to_string));
386+
return TrustPlan::Scaffold(lines.join("\n"));
387+
}
343388
// Where the key would go — refused when the document is not a single
344389
// block mapping, so the scan for an existing key below is meaningful.
345390
let anchor = match block_insert_point(&lines) {

‎crates/socket-patch-core/src/patch/redirect/upstream/npm.rs‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2081,8 +2081,15 @@ pub(crate) async fn cleanup_side_config(
20812081
if restored_pnpm && !still_hosted(view, &["pnpm-lock.yaml"], ctx).await {
20822082
const WORKSPACE: &str = "pnpm-workspace.yaml";
20832083
if let Ok(Some(ws)) = view.read(WORKSPACE).await {
2084-
if ws == "packages:\n - '.'\ntrustLockfile: true\n" {
2084+
let lines = crate::hosted::guidance::TRUST_SCAFFOLD_LINES;
2085+
if ws == format!("{}\n", lines.join("\n")) {
20852086
view.remove(WORKSPACE);
2087+
} else if let Some(original) =
2088+
crate::formats::pnpm::workspace::strip_keyless_splice(&ws, &lines)
2089+
{
2090+
// The scaffold hosted mode splices into a file with no keys
2091+
// (#1096): taking it back out restores the user's file.
2092+
view.write(WORKSPACE, original);
20862093
} else if ws.lines().any(|l| {
20872094
crate::formats::pnpm::workspace::top_level_key(l).is_some_and(|(key, value)| {
20882095
key == "trustLockfile" && value.trim_matches(['\'', '"']) == "true"

0 commit comments

Comments
 (0)