From 18c9ab1c55caf3f480c88dbf1651cfaa38ed8fec Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 1 Nov 2026 22:23:23 +0000 Subject: [PATCH 1/2] Start fix for #1949, #1952 Assisted-by: Claude Code:claude-opus-5-5 From 48e8e6c8d9e754086f4d4772a9bf2cb69222d37f Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 00:26:36 +0000 Subject: [PATCH 2/2] Read pnpm locks' project document in every reader pnpm 11+ writes its env lockfile (configDependencies / packageManagerDependencies) as a first YAML document. PnpmLock::parse walked the whole text, so the scan inventory listed config dependencies the install never links while InstalledPackages did not (#1949). It now keeps only entries that start in grammar::main_document, with offsets still relative to the full text, and reads vendored uuids and the packages: sniff from that document. "Does the root lock list a member importer?" was answered by two drifted copies: the formats one read a commented-out ` # sub:` key as a member, the utils one did not, and neither isolated the main document itself (#1952). One predicate in formats::pnpm::workspace now skips comment lines (a column-0 comment no longer ends importers:), and lock_text_has_member_importers applies main_document; the utils copy and the engine's inline wrapper route through it. Fixes #1949 Fixes #1952 Co-Authored-By: Claude --- .../socket-patch-core/src/formats/pnpm/mod.rs | 66 +++++++++++++++++-- .../src/formats/pnpm/workspace.rs | 52 +++++++++++++-- crates/socket-patch-core/src/hosted/engine.rs | 9 +-- .../src/utils/pnpm_workspace.rs | 50 ++++---------- 4 files changed, 125 insertions(+), 52 deletions(-) diff --git a/crates/socket-patch-core/src/formats/pnpm/mod.rs b/crates/socket-patch-core/src/formats/pnpm/mod.rs index 2bfd3033c..699fc0473 100644 --- a/crates/socket-patch-core/src/formats/pnpm/mod.rs +++ b/crates/socket-patch-core/src/formats/pnpm/mod.rs @@ -369,8 +369,13 @@ pub(crate) fn check_v9_lock_version(text: &str) -> Result<(), String> { /// One pnpm lock, parsed once (see the module docs). pub struct PnpmLock<'t> { text: &'t str, + /// The project document ([`grammar::main_document`]): pnpm 11+ writes + /// its env lockfile (`configDependencies`, `packageManagerDependencies`) + /// as a first document whose packages the install never links (#1949). + doc: &'t str, + /// Entries of the project document only, with offsets into `text`. packages: Vec>, - /// [`vendored_npm_uuids`] of the text. + /// [`vendored_npm_uuids`] of the project document. vendored: HashSet, } @@ -386,10 +391,18 @@ pub(crate) struct PnpmTarballRef<'p> { impl<'t> PnpmLock<'t> { /// Parse a lock text (any content: a non-pnpm text has no entries). pub fn parse(text: &'t str) -> Self { + let doc = grammar::main_document(strip_bom(text)); + // `doc` is a suffix of `text`: parse the whole text so offsets stay + // relative to it, and keep the entries that start in `doc`. + let doc_start = text.len() - doc.len(); PnpmLock { text, - packages: pnpm_packages(text), - vendored: vendored_npm_uuids(text), + doc, + packages: pnpm_packages(text) + .into_iter() + .filter(|p| p.entry.offset >= doc_start) + .collect(), + vendored: vendored_npm_uuids(doc), } } @@ -408,7 +421,7 @@ impl<'t> PnpmLock<'t> { } fn has_packages_section(&self) -> bool { - self.text + self.doc .lines() .any(|l| l.trim_end_matches('\r') == "packages:") } @@ -754,6 +767,51 @@ mod tests { assert!(!PnpmLock::parse(importers).resolves("left-pad", "1.3.0")); } + /// #1949: a pnpm 11+ lock's env document (config dependencies the + /// install never links) is not part of the inventory: `entries()`, + /// `resolves()`, the vendored uuids and `InstalledPackages` all read + /// the project document alone, and agree with each other. + #[test] + fn env_document_is_not_in_the_inventory() { + let env = format!( + "---\nlockfileVersion: '9.0'\n\nimporters:\n\n .:\n configDependencies:\n cfg-dep:\n specifier: 2.0.0\n version: 2.0.0\n\npackages:\n\n cfg-dep@2.0.0:\n resolution: {{integrity: sha512-c}}\n\n b@file:.socket/vendor/npm/{UUID}/b-1.0.0.tgz:\n resolution: {{integrity: sha512-x, tarball: file:.socket/vendor/npm/{UUID}/b-1.0.0.tgz}}\n version: 1.0.0\n\n" + ); + let project = "---\nlockfileVersion: '9.0'\n\nimporters:\n\n .:\n dependencies:\n left-pad:\n specifier: 1.3.0\n version: 1.3.0\n\npackages:\n\n left-pad@1.3.0:\n resolution: {integrity: sha512-x}\n"; + for text in [ + format!("{env}{project}"), + format!("\u{feff}{env}{project}"), + format!("{env}{project}").replace('\n', "\r\n"), + ] { + let lock = PnpmLock::parse(&text); + let names: Vec<_> = lock + .entries() + .unwrap() + .into_iter() + .map(|e| (e.name, e.version)) + .collect(); + assert_eq!( + names, + [("left-pad".to_string(), "1.3.0".to_string())], + "{text:?}" + ); + assert!(lock.resolves("left-pad", "1.3.0")); + assert!(!lock.resolves("cfg-dep", "2.0.0")); + assert!(!lock.vendored_in_use(UUID)); + assert!(lock.packages().iter().all(|p| p.entry.offset >= env.len())); + let installed = InstalledPackages::from_lock_text(&text).unwrap(); + assert!(installed.contains("left-pad", "1.3.0")); + assert!(!installed.contains("cfg-dep", "2.0.0")); + } + // Offsets still index the whole text. + let text = format!("{env}{project}"); + let lock = PnpmLock::parse(&text); + let entry = &lock.packages()[0].entry; + assert_eq!( + &text[entry.offset..entry.offset + entry.body.len()], + entry.body + ); + } + #[test] fn vendored_in_use_reads_v9_and_legacy_keys_and_crlf() { let v9 = format!( diff --git a/crates/socket-patch-core/src/formats/pnpm/workspace.rs b/crates/socket-patch-core/src/formats/pnpm/workspace.rs index ede03b140..afb978244 100644 --- a/crates/socket-patch-core/src/formats/pnpm/workspace.rs +++ b/crates/socket-patch-core/src/formats/pnpm/workspace.rs @@ -151,14 +151,20 @@ pub(crate) fn is_keyless(text: &str) -> bool { /// 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 { +/// [`PACKAGES_SCAFFOLD`] would drop those members. The same answer marks a +/// root lock as a SHARED workspace lock (`utils::pnpm_workspace`). Comment +/// lines are neither keys nor section ends (#1952). +pub(crate) fn lock_has_member_importers>(lines: &[S]) -> bool { // CRLF-blind: a Windows checkout's lock reads like its LF twin. let mut in_importers = false; for line in lines { + let line = line.as_ref(); let line = line.strip_suffix('\r').unwrap_or(line); - if !line.is_empty() && !line.starts_with(' ') { - in_importers = line == "importers:"; + if line.trim_start().starts_with('#') { + continue; + } + if !line.is_empty() && !line.starts_with([' ', '\t']) { + in_importers = line.trim_end() == "importers:"; } else if in_importers && super::lines::parse_key_line(line, 2).is_some_and(|(key, _, _)| key != ".") { @@ -168,6 +174,14 @@ pub(crate) fn lock_has_member_importers(lines: &[String]) -> bool { false } +/// [`lock_has_member_importers`] of a whole `pnpm-lock.yaml` text: its +/// project document alone ([`super::grammar::main_document`]), so importers +/// of a pnpm 11+ env document never count. +pub(crate) fn lock_text_has_member_importers(text: &str) -> bool { + let doc = super::grammar::main_document(strip_bom(text)); + lock_has_member_importers(&doc.split('\n').collect::>()) +} + /// `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 @@ -506,6 +520,36 @@ mod tests { assert!(lock_has_member_importers(&crlf)); } + /// #1952: a commented-out importer is not a member, a column-0 comment + /// does not end `importers:`, and the env document's importers never + /// count; the line and text forms agree. + #[test] + fn member_importers_skip_comments_and_the_env_document() { + let head = "lockfileVersion: '9.0'\n\nimporters:\n\n .:\n dependencies: {}\n"; + for (body, members) in [ + (" # sub:\n # dependencies: {}\n", false), + ("# a note\n sub:\n dependencies: {}\n", true), + (" # nested: comment\n", false), + ] { + let text = format!("{head}{body}\npackages: {{}}\n"); + assert_eq!(lock_text_has_member_importers(&text), members, "{text:?}"); + assert_eq!( + lock_has_member_importers(&lines(&text)), + members, + "{text:?}" + ); + } + let env = "---\nlockfileVersion: '9.0'\n\nimporters:\n\n .: {}\n\n tooling:\n configDependencies: {}\n\n"; + let project = format!("---\n{head}\npackages: {{}}\n"); + assert!(!lock_text_has_member_importers(&format!("{env}{project}"))); + assert!(!lock_text_has_member_importers(&format!( + "\u{feff}{env}{project}" + ))); + assert!(lock_text_has_member_importers(&format!( + "{env}---\n{head} sub:\n dependencies: {{}}\n" + ))); + } + #[test] fn keyless_documents_and_their_splices() { for text in [ diff --git a/crates/socket-patch-core/src/hosted/engine.rs b/crates/socket-patch-core/src/hosted/engine.rs index f214e686d..2599667c0 100644 --- a/crates/socket-patch-core/src/hosted/engine.rs +++ b/crates/socket-patch-core/src/hosted/engine.rs @@ -2991,13 +2991,8 @@ fn read_advisory_text(view: &ProjectView<'_>, rel: &str) -> Option { /// root: with a keyless pnpm-workspace.yaml that marks a pnpm <= 10.4 /// install (see [`crate::formats::pnpm::workspace::lock_has_member_importers`]). fn lock_lists_members(view: &ProjectView<'_>) -> bool { - read_advisory_text(view, "pnpm-lock.yaml").is_some_and(|text| { - let doc = - crate::formats::pnpm::grammar::main_document(crate::formats::text::strip_bom(&text)); - crate::formats::pnpm::workspace::lock_has_member_importers( - &crate::formats::pnpm::lines::split_lines(doc), - ) - }) + read_advisory_text(view, "pnpm-lock.yaml") + .is_some_and(|text| crate::formats::pnpm::workspace::lock_text_has_member_importers(&text)) } /// The ancestor `pnpm-workspace.yaml` governing a disk project's pnpm diff --git a/crates/socket-patch-core/src/utils/pnpm_workspace.rs b/crates/socket-patch-core/src/utils/pnpm_workspace.rs index b12928665..3fbcc1f4b 100644 --- a/crates/socket-patch-core/src/utils/pnpm_workspace.rs +++ b/crates/socket-patch-core/src/utils/pnpm_workspace.rs @@ -262,37 +262,6 @@ pub fn shared_lockfile_disabled(workspace: &str, npmrc: Option<&str>) -> bool { .is_some_and(|setting| setting.as_bool() == Some(false)) } -/// Whether a root lock is a SHARED workspace lock: its `importers:` names a -/// project other than the root (`.`). Such a lock is what pnpm installs -/// every member from, whatever the settings say now, so member locks -/// beside it are stale leftovers. -pub fn root_lock_lists_members(lock: &str) -> bool { - let mut in_importers = false; - for line in lock.lines() { - let line = line.strip_suffix('\r').unwrap_or(line); - if !line.starts_with([' ', '\t']) && !line.trim().is_empty() { - in_importers = line.trim_end() == "importers:"; - continue; - } - if !in_importers { - continue; - } - // An importer key sits at exactly two spaces. - let Some(key) = line - .strip_prefix(" ") - .filter(|k| !k.starts_with([' ', '#'])) - else { - continue; - }; - let key = key.split(':').next().unwrap_or("").trim(); - let key = key.trim_matches(['\'', '"']); - if !key.is_empty() && key != "." { - return true; - } - } - false -} - /// Whether pnpm's project finder lists the directory at `rel` (relative /// to the workspace root, one entry per component) under `globs` /// (`pnpm-workspace.yaml` `packages:`, as [`read_package_globs`] reads them), as @@ -473,7 +442,13 @@ async fn workspace_members(view: &ProjectView<'_>) -> Members { return Members::Shared; } let root_lock = view.read_text(PNPM_LOCK).await.ok(); - if root_lock.as_deref().is_some_and(root_lock_lists_members) { + // A root lock whose importers name a project other than `.` is a + // SHARED workspace lock: pnpm installs every member from it, whatever + // the settings say now, so member locks beside it are stale leftovers. + if root_lock + .as_deref() + .is_some_and(crate::formats::pnpm::workspace::lock_text_has_member_importers) + { return Members::Shared; } let globs = match read_package_globs(&workspace) { @@ -1217,18 +1192,19 @@ mod tests { #[test] fn a_root_lock_listing_member_importers_is_shared() { + use crate::formats::pnpm::workspace::lock_text_has_member_importers; let root_only = "lockfileVersion: '9.0'\n\nimporters:\n\n .:\n dependencies:\n a:\n specifier: 1.0.0\n version: 1.0.0\n\npackages:\n\n a@1.0.0:\n resolution: {integrity: sha512-x}\n"; - assert!(!root_lock_lists_members(root_only)); - assert!(!root_lock_lists_members( + assert!(!lock_text_has_member_importers(root_only)); + assert!(!lock_text_has_member_importers( "lockfileVersion: '9.0'\n\nimporters:\n\n .: {}\n" )); - assert!(!root_lock_lists_members( + assert!(!lock_text_has_member_importers( "lockfileVersion: 5.4\n\ndependencies:\n a: 1.0.0\n" )); - assert!(root_lock_lists_members( + assert!(lock_text_has_member_importers( "lockfileVersion: '9.0'\r\n\r\nimporters:\r\n\r\n .: {}\r\n\r\n packages/a:\r\n dependencies: {}\r\n" )); - assert!(root_lock_lists_members( + assert!(lock_text_has_member_importers( "lockfileVersion: '6.0'\nimporters:\n '.': {}\n 'packages/a': {}\n" )); }