Skip to content

Commit f54766b

Browse files
Fix pnpm brace and class workspace globs (#1846) (#1863)
* Start fix for #1846 Assisted-by: Claude Code:claude-opus-5-5 * Skip comments when reading pnpm lock importers The check for "does this pnpm lock list workspace members" counted a `#`-commented importer line as a member, and a column-0 comment ended the importers section early. It now skips comments, accepts tab-led lines, and gains a whole-text entry point so the second, hand-split copy in pnpm_workspace can go (#1846). Assisted-by: Claude Code:claude-opus-5-5 * Read pnpm brace and class workspace globs A pnpm-workspace.yaml listing members as `packages/{a,b}` or `packages/[ab]*` was refused by every non-hosted membership check, or (when deciding which workspace file governs a project) treated as listing every directory, even though pnpm installs exactly those members and the shared glob matcher already reads that syntax. Hosted mode kept its own second reader to work around the refusal. Now one reader answers for all callers: brace sets and plain classes of letters, digits, `_` and `-` match as pnpm 10.28 reads them, and the member walk expands braces and widens classes before narrowing. Syntax pnpm reads its own way stays refused: `[!a]` (pnpm reads it as `[a]`), `[^a]` and `[.]` (they match a leading dot), POSIX classes and extglob groups. The hosted fallback reader and the duplicate lock-importer detector are gone (#1846). Assisted-by: Claude Code:claude-opus-5-5 * Use an extglob as the unmodeled pnpm glob The hosted/in-memory parity test used `packages/{a,b}` as its example of a pnpm glob socket-patch cannot read. Brace sets are now read (#1846), so the case uses an extglob group, which is still refused. Assisted-by: Claude Code:claude-opus-5-5 * Refuse pnpm globs pnpm reads as regex or escape Review on #1846: beside a class or brace set, pnpm reads `+` as a quantifier (`packages/[ab]+` lists `packages/ab`) and `\` as an escape (`packages/\[a]` lists only a directory named `[a]`), while the shared matcher reads `+` literally and `\` as a separator. Those globs are refused again, in each brace alternative too, instead of answering "not a member". Globs without classes or braces read as before. The member walk now applies one directory bound across every brace alternative, so a brace glob can no longer collect many times more candidates than the limit. Each candidate also skips the repeated syntax check. Assisted-by: Claude Code:claude-opus-5-5 * Refuse pnpm class ranges that start at a dash A class range from `-` (as in `packages/[--9]h`) spans `.` and `/`, so pnpm can match a hidden directory that the shared matcher never does. Such classes are refused now. A literal `-` (`[-a]`) and ranges between letters, digits and `_` stay readable. Assisted-by: Claude Code:claude-opus-5-5 * Refuse pnpm brace shapes that pnpm reads differently pnpm 10.28.0 reads braces as picomatch does, not as brace-expansion does. So modeled() accepted three shapes whose members differ from what pnpm lists: - a stepped or multi-character sequence: `p{1..10}` lists p0 and p1 under pnpm, not p10; - a `*` or `?` inside braces: `{a,*}` lists dot directories under pnpm; - an empty alternative: `packages/{a,}` listed `packages` itself. Refuse these shapes, as the rest of modeled() already does for syntax pnpm reads its own way, instead of guessing a member set that would reach member_dirs and the hosted governing root. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * Cap the brace alternatives pnpm member_dirs walks member_dirs walks the tree once for each brace alternative, and the directory bound only counts matches, so an alternative that matches nothing walks the whole tree without ever reaching it. A scanned repo's pnpm-workspace.yaml could list up to 4095 alternatives (for example **/{a..z}{a..z}{a..e}) and stall the scan for minutes on a modest tree. Allow at most 64 alternatives over all globs and answer Err beyond that, which callers already treat as no answer. Refs #1846 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011f9aeRR24V93EPLyuXR8tp * Count only brace walks against pnpm walk cap The cap on how many tree walks a pnpm-workspace.yaml can ask for also counted globs without braces. A workspace listing more than 64 plain `packages:` entries, which large monorepos do, failed member-lock discovery, though main walks those without a cap. Only the extra walks brace sets add now count; a plain glob is one walk, as before. Assisted-by: Claude Code:claude-opus-5-5 * Refuse pnpm brace ranges across punctuation A brace sequence with single-character ends that are not the same kind stepped through punctuation: `packages/{-..0}` expanded through `.` and `/`, listing `packages` itself as a member, and `{0..Z}` through `?`, which the walk then read as a wildcard. A sequence now needs both ends to be digits, both lowercase or both uppercase. Found in the final review of #1863. Assisted-by: Claude Code:claude-opus-5-5 * Refuse pnpm brace sets pnpm reads as sequences or class members pnpm 10.28 expands a `..` inside a comma brace set as a sequence (`packages/{a,b..c}` lists `,` `0` `A` `[` ...), and reads braces inside a class as class members (`packages/[a{b,c}]` lists `,` `{` `}`). modeled() accepted both and listed a different member set, where the rule is to refuse syntax we don't read the way pnpm does. Refuse any alternative holding `..` beside a comma, and check classes on the raw glob too. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011f9aeRocMg7S8GHB43bJsX * Test pnpm brace groups without a comma stay literal pnpm 10.28 keeps a brace group with no top-level comma or `..` literal, also around a nested set: `packages/{app-{web,api}}` lists only a directory named `{app-web}`, never `app-web`. The shared matcher reads it the same way; this pins that. Assisted-by: Claude Code:claude-opus-5-5 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 81ae437 commit f54766b

4 files changed

Lines changed: 432 additions & 55 deletions

File tree

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1616,7 +1616,8 @@ async fn an_unconfirmed_pnpm_member_keeps_its_lock() {
16161616
let server = pnpm_server().await;
16171617
let per_member = "packages:\n - 'packages/*'\nsharedWorkspaceLockfile: false\n";
16181618
let unmodeled = pnpm_workspace(
1619-
"packages:\n - 'packages/{a,b}'\nsharedWorkspaceLockfile: false\n",
1619+
// Brace sets and classes are read (#1846); an extglob is not.
1620+
"packages:\n - 'packages/@(a|b)'\nsharedWorkspaceLockfile: false\n",
16201621
Some(PNPM_ROOT_ONLY_LOCK),
16211622
&["packages/a"],
16221623
&[],

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

Lines changed: 48 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -693,10 +693,12 @@ async fn lockless_npm_project(base: &Path, mut dir: PathBuf) -> Option<PathBuf>
693693
///
694694
/// Unlike the hosted refusals, this errs toward "not a member": a member's
695695
/// copies are in its workspace's lock, so they never get this far, while a
696-
/// project wrongly kept the scan root's fails that root's run. pnpm globs
697-
/// the pnpm matcher does not model (braces, classes) go through the
698-
/// shared workspace grammar; a `pnpm-workspace.yaml` that cannot be read,
699-
/// which pnpm cannot install from either, lists nothing.
696+
/// project wrongly kept the scan root's fails that root's run. So a
697+
/// `pnpm-workspace.yaml` that cannot be read, which pnpm cannot install
698+
/// from either, or whose `packages:` use glob syntax the pnpm reader does
699+
/// not model ([`lists_member`]), lists nothing.
700+
///
701+
/// [`lists_member`]: crate::utils::pnpm_workspace::lists_member
700702
async fn listing_workspace(base: &Path, dir: &Path) -> Option<PathBuf> {
701703
let lists = |patterns: &[String], rel: &[String]| {
702704
WorkspacesDialect::ALL
@@ -730,8 +732,7 @@ async fn listing_workspace(base: &Path, dir: &Path) -> Option<PathBuf> {
730732
.ok()
731733
.and_then(|yaml| read_package_globs(&yaml).ok().flatten())
732734
.is_some_and(|globs| {
733-
crate::utils::pnpm_workspace::lists_member(&globs, &rel)
734-
.unwrap_or_else(|_| pnpm_lists_unmodeled(&globs, &rel))
735+
crate::utils::pnpm_workspace::lists_member(&globs, &rel).unwrap_or(false)
735736
});
736737
if listed {
737738
return Some(ancestor.to_path_buf());
@@ -740,31 +741,6 @@ async fn listing_workspace(base: &Path, dir: &Path) -> Option<PathBuf> {
740741
None
741742
}
742743

743-
/// Whether pnpm lists `rel` under `packages:` globs that use syntax its
744-
/// own matcher here does not model (braces, classes): the shared workspace
745-
/// grammar matches each glob, read pnpm's way. A `!` glob excludes whatever
746-
/// its order, wildcards never match a dot directory, and `node_modules`,
747-
/// `test` and `tests` directories are never members.
748-
fn pnpm_lists_unmodeled(globs: &[String], rel: &[String]) -> bool {
749-
use crate::utils::workspace_globs::Negation;
750-
const PNPM: WorkspacesDialect = WorkspacesDialect {
751-
negation: Negation::LastMatchWins,
752-
extglob: false,
753-
picomatch: false,
754-
dot: false,
755-
};
756-
if rel
757-
.iter()
758-
.any(|seg| crate::utils::pnpm_workspace::MEMBER_SKIP.contains(&seg.as_str()))
759-
{
760-
return false;
761-
}
762-
let matches = |glob: &str| workspaces_include_as(&[glob.to_string()], rel, PNPM);
763-
let (negated, listed): (Vec<&String>, Vec<&String>) =
764-
globs.iter().partition(|glob| glob.starts_with('!'));
765-
listed.iter().any(|glob| matches(glob)) && !negated.iter().any(|glob| matches(&glob[1..]))
766-
}
767-
768744
/// The hosted refusal for [`member_stray_lock`].
769745
async fn member_stray_lock_refusal(root: &Path) -> Option<(PathBuf, Refusal)> {
770746
let stray = member_stray_lock(root).await?;
@@ -1388,6 +1364,47 @@ mod tests {
13881364
);
13891365
}
13901366

1367+
/// #1846: hosted reads pnpm brace and class globs through the same
1368+
/// reader as `pnpm_workspace::member_dirs`, so both name the same
1369+
/// members; syntax that reader refuses lists nothing here.
1370+
#[tokio::test]
1371+
async fn pnpm_brace_and_class_globs_list_what_member_dirs_lists() {
1372+
use crate::utils::cargo_workspace::DiskTree;
1373+
let dirs = ["a", "b", "c", "ab"];
1374+
for (glob, listed) in [
1375+
("packages/{a,b}", &["packages/a", "packages/b"][..]),
1376+
(
1377+
"packages/[ab]*",
1378+
&["packages/a", "packages/ab", "packages/b"],
1379+
),
1380+
("packages/[!a]*", &[]),
1381+
] {
1382+
let tmp = tempfile::tempdir().unwrap();
1383+
let root = tmp.path();
1384+
write(root, "package.json", r#"{"name":"root"}"#);
1385+
write(root, PNPM_WORKSPACE, &format!("packages:\n - '{glob}'\n"));
1386+
for dir in dirs {
1387+
write(root, &format!("packages/{dir}/package.json"), "{}");
1388+
}
1389+
let globs = vec![glob.to_string()];
1390+
let members = crate::utils::pnpm_workspace::member_dirs(&DiskTree(root), &globs);
1391+
if listed.is_empty() {
1392+
assert!(members.is_err(), "{glob}");
1393+
} else {
1394+
assert_eq!(members.unwrap(), listed, "{glob}");
1395+
}
1396+
for dir in dirs {
1397+
let rel = format!("packages/{dir}");
1398+
let expected = listed.contains(&rel.as_str()).then(|| root.to_path_buf());
1399+
assert_eq!(
1400+
listing_workspace(root, &root.join(&rel)).await,
1401+
expected,
1402+
"{glob} {rel}"
1403+
);
1404+
}
1405+
}
1406+
}
1407+
13911408
/// #1554: a project installed without a lock (`package-lock=false`,
13921409
/// `npm install --no-package-lock`) is independent too, unless a
13931410
/// workspace lists it, whether or not that workspace was ever locked.

0 commit comments

Comments
 (0)