GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — Fix pnpm brace and class workspace globs (#1846) by mikolalysenko · Pull Request #1863 · SocketDev/socket-patch

Conversation

Mikola Lysenko (mikolalysenko) commented Oct 29, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Collaborator

LLM Description written by Claude Code:claude-opus-5-5

Fixes #1846

Summary

A pnpm-workspace.yaml listing members as packages/{a,b} or packages/[ab]* was refused by every non-hosted membership check (member_dirs, root_accounts_for_member_lock). When working out which workspace file governs a project, it was instead treated as listing every directory. pnpm installs exactly those members, and the shared glob matcher already reads that syntax. Hosted mode had its own second reader to work around the refusal, so hosted and non-hosted answered differently.

User-visible: brace sets and plain character classes in packages: are no longer refused outside hosted mode. They now resolve to the members pnpm itself lists.

Root cause

utils::pnpm_workspace::modeled refused any glob containing { } [ ] ( ) as "syntax the shared matcher does not model". But workspace_globs::pattern_matches already expands braces and matches classes. In addition, member_dirs walked candidates with cargo's expand_glob_bounded, which reads neither braces nor classes.

Changes

modeled now refuses only syntax that pnpm reads differently from the shared matcher. I probed this on pnpm 10.28.0 with pnpm list -r, over packages/{a,b,c,ab,x,.h}:

glob	pnpm lists	here
`packages/{a,b}`	a b	read
`packages/[ab]*`	a ab b	read
`packages/{a..c}`	a b c	read
`packages/[a-b]`	a b	read
`packages/*`, `!packages/[ab]`	ab c x	read
`packages/[!a]*`	a ab (pnpm reads `[!a]` as `[a]`)	refused
`packages/[^a]*`	.h b c x (matches a leading dot)	refused
`packages/[.]h`	.h	refused
`packages/[[:alpha:]]`	a b c x	refused
`packages/@(a	b)`,` packages/(a	b)`

A class is read only when it is closed, not negated, and holds just letters, digits, _ and - ranges. Such a class can never match . or /. A glob with more brace alternatives than the matcher expands is also refused, so the answer is never partial.

member_dirs walks each brace alternative, with its classes widened to ? (a superset), and then lists_member narrows the result exactly.
hosted::governing_root::pnpm_lists_unmodeled and its unwrap_or_else fallback are deleted. Hosted now reads through lists_member. Syntax that is still unmodeled lists nothing there, which matches that function's documented "errs toward not a member" rule.
There is now one detector for "the lock lists member importers": formats::pnpm::workspace, with a new lock_text_has_member_importers(&str) entry point. It replaces pnpm_workspace::root_lock_lists_members. It now skips # comment lines: an indented commented importer was counted as a member, and a column-0 comment ended the section early. It also treats tab-led lines as continuation, as the deleted copy did.
Brace shapes that pnpm reads as picomatch does, not as brace expansion, are refused (a7ee393, from the final review). These are stepped or multi-character sequences (p{1..10} lists p0 p1 under pnpm), * or ? inside braces (these match a leading .), and an empty alternative (packages/{a,} must not list packages).
member_dirs walks at most 64 brace alternatives across all globs and returns Err beyond that (2161606, from the final review). Globs without braces are not counted against that cap (d52f3b6, from Bugbot). Each alternative is one walk of the tree, and an alternative that matches nothing never reaches MAX_MANIFESTS. In the reviewer's measurement, **/{a..z}{a..z}{a..e} took 519 s on about 4,200 directories.
A {x..y} sequence must have both ends digits, both lowercase or both uppercase (5ca5e0e, from the final review's open question): {-..0} stepped through . and /, and {0..Z} through ?.
A .. beside a comma in a brace set ({a,b..c}) and braces inside a class ([a{b,c}]) are refused (032b960, from the final review): pnpm reads the first as a sequence and the second as class members.
Merged main after the merge queue dropped this PR for a conflict (6dc33e2). main's "pnpm lock importers list a non-root member" is written twice and drifted on comment lines: keep formats::pnpm::workspace, delete the utils::pnpm_workspace copy #1952 had already merged the two lock-importer detectors, so this PR now uses main's lock_has_member_importers / lock_text_has_member_importers and no longer touches formats/pnpm/workspace.rs.
Behaviour notes: the merged lock-importer reader skips # comments, no longer ends importers: at a column-0 comment, and treats tab-indented lines as indented. The member walk's MAX_MANIFESTS bound now counts the union of every glob's candidates.
I did not add WorkspacesDialect::PNPM, which the issue suggested. With the second reader gone, lists_member (via glob_matches_no_dot) is the only pnpm reader, so the constant would have no caller.

Test evidence

Red before the fix (cargo test -p socket-patch-core --lib, new tests on 78ccde1 plus the test commit): 4 failed: member_dirs_read_braces_and_classes_as_pnpm_does, a_project_outside_the_packages_globs_is_not_governed, member_importers_mark_a_pre_10_5_workspace (commented importer), lock_text_member_importers_read_every_lock_shape.

Green after:

cargo fmt --all -- --check: ok
cargo clippy --workspace --all-features -- -D warnings: ok
cargo test -p socket-patch-core --all-features: 6992 passed. 4 failed only because this container runs as root, so chmod-based "write fails" tests cannot fail: copy_tree::relax_loop_must_not_traverse_symlinked_root, vlt_heal::an_unremovable_hidden_lock_keeps_every_store_entry, pypi_poetry::wire_write_failure_…, pypi_requirements::wire_failure_rolls_back_…. This PR doesn't touch those files.
CLI: hosted_memory_parity (42), in_process_redirect_pnpm (41), in_process_vendor_pnpm_parent_child (8), in_process_vendor_pnpm_takeover (15), scan_pnpm_relocated_store_cwd_e2e (23): all pass.
e2e_redirect_pnpm_build -- --ignored --skip pnpm_pinned_matrix (CI's leg, local pnpm 10.28): 11 passed.
The full cargo test --workspace run ran out of container disk while linking CLI test binaries, so CI is the full run.

Per-issue checklist (#1846 acceptance criteria):

Brace and class workspaces resolve the same members in hosted and member_dirs: governing_root::tests::pnpm_brace_and_class_globs_list_what_member_dirs_lists, pnpm_workspace::tests::member_dirs_read_braces_and_classes_as_pnpm_does (disk and in-memory trees).
A #-commented importer line is not a member: member_importers_mark_a_pre_10_5_workspace, lock_text_member_importers_read_every_lock_shape.
Existing pnpm workspace and governing_root tests pass. hosted_memory_parity::an_unconfirmed_pnpm_member_keeps_its_lock now uses an extglob as its unmodeled glob, because braces are read now.

/code-review high

Fixed in acf9d23:

+ and \ beside a class or brace set: pnpm reads them as a quantifier and an escape. I confirmed this on pnpm 10.28: packages/[ab]+ lists a, ab, b; packages/\[a] lists only [a]. These globs are refused again, checked on the raw glob and on each brace alternative (so {Z..a} is refused too).
The walk bound now covers all brace alternatives together, not each one separately (new test in member_dirs_refuse_a_glob_the_bounded_walk_cannot_finish).
member_dirs no longer re-runs modeled for each candidate (lists_modeled_member).
A glob with too many brace alternatives returns an Err from member_dirs instead of relying on unwrap_or_default.
The Root scan --mode vendored still exits 1 forever and drops its in-run VEX when a nested npm project has no lockfile (package-lock=false); the #1534 fix only covers nested projects with their own lock #1554 doc comment is back on its own test.

Bugbot (acf9d23): a class range starting at - ([--9]) spans . and /. Fixed in f9b15ba: a class body containing -- is refused.

Not fixed, by choice:

Teach cargo_workspace::expand_glob_bounded braces and classes for every caller. That is Cargo workspace member globs ignore [...] character classes, so members = ["crates/[ab]*"] expands to no members #1833 (Cargo classes, p2), a separate root cause. Here the widen-then-narrow walk stays local to pnpm and stays exact, because lists_member decides every candidate.
Build modeled's class check on workspace_globs::char_class. The check here deliberately accepts only a strict subset: closed, non-negated classes of [A-Za-z0-9_-] with no range starting at -. Anything else, []a] included, is refused. So when it disagrees with the matcher's grammar, the result is a refusal, never a wrong answer.

https://claude.ai/code/session_011f9aeQPdgsuof1CE3TcTC5

Generated by Claude Code

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          Start fix for #1846

78ccde1

Assisted-by: Claude Code:claude-opus-5-5

Mikola Lysenko (mikolalysenko) mentioned this pull request

Oct 29, 2026

pnpm workspace membership refuses brace and class globs the shared matcher already reads, and hosted keeps a second reader for them #1846

Closed

Claude (claude) added 4 commits

October 29, 2026 22:37


          Skip comments when reading pnpm lock importers

77724bc

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

4b0f952

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

9ed90e7

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

acf9d23

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

Mikola Lysenko (mikolalysenko) marked this pull request as ready for review

October 29, 2026 23:04

Mikola Lysenko (mikolalysenko) commented Oct 29, 2026

Copy link

Copy Markdown

Collaborator Author

BugBot review

Generated by Claude Code

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

1 similar comment

Mikola Lysenko (mikolalysenko) commented Oct 29, 2026

Copy link

Copy Markdown

Collaborator Author

BugBot review

Generated by Claude Code

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

cursor Bot reviewed

Oct 29, 2026

View reviewed changes

cursor Bot left a comment •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

crates/socket-patch-core/src/utils/pnpm_workspace.rs Resolved

Uh oh!

There was an error while loading. Please reload this page.


          Refuse pnpm class ranges that start at a dash

f9b15ba

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

Mikola Lysenko (mikolalysenko) commented Oct 29, 2026

Copy link

Copy Markdown

Collaborator Author

BugBot review

Generated by Claude Code

cursor[bot] reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

cursor Bot reviewed

Oct 29, 2026

View reviewed changes

cursor Bot left a comment •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Mikola Lysenko (mikolalysenko) added the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label

Oct 30, 2026

Mikola Lysenko (mikolalysenko) commented Oct 30, 2026

Copy link

Copy Markdown

Collaborator Author

Burn-down agent: labeled Ready for review at f9b15ba. CI 50/50 green on this head; mergeable, no conflicts. Bugbot reviewed f9b15ba: no findings; no open review threads. Slack announcement not sent this run (Slack posting unavailable).

Generated by Claude Code

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Mikola Lysenko (mikolalysenko) commented Oct 30, 2026

Copy link

Copy Markdown

Collaborator Author

Burn-down agent: labeled Ready for review at f9b15ba3eeec7c3afd07b9169294f1916b5fc240.

CI: 31/31 run checks green (19 skipped by path filters); no main-wide failures.
Bugbot: reviewed f9b15ba3ee, no new issues; no unresolved review threads.
Mergeable: yes, no conflicts; waiting only on human approval.
Slack: not announced (Slack send unavailable in this run); the next run retries.

Generated by Claude Code

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Mikola Lysenko (mikolalysenko) mentioned this pull request

Oct 30, 2026

Swarm health manager log #1382

Open


          Refuse pnpm brace shapes that pnpm reads differently

a7ee393

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) <[email protected]>

Mikola Lysenko (mikolalysenko) removed the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label

Oct 30, 2026

Mikola Lysenko (mikolalysenko) commented Oct 30, 2026

Copy link

Copy Markdown

Collaborator Author

Final review of f9b15ba: I found three brace shapes that modeled() accepted but pnpm lists differently. I pushed a7ee393, which refuses them.

I compared against a real pnpm 10.28.0 (pnpm ls -r --depth -1 --json on a fixture tree), which reads braces as picomatch does:

Stepped or multi-character sequences. For packages/p{1..10}, pnpm lists p0 p1; the PR listed p1 p2 p10. {a..e..2} and p{01..2} also differed. Unstepped single-character sequences ({a..c}, {c..a}, {0..2}) agree and stay accepted.
* or ? inside braces. pnpm's {a,*} and {?h,x} match a leading . (packages/.h); the PR's matcher doesn't.
An empty alternative. For packages/{a,} and {,a}, the PR listed packages itself; pnpm lists only packages/a.

The fix adds plain_braces in pnpm_workspace.rs and new refusal cases to member_dirs_read_braces_and_classes_as_pnpm_does. The test fails without the fix.

Ran: cargo test -p socket-patch-core --lib filtered to pnpm, workspace_globs and governing_root (511 pass), and hosted_memory_parity (42 pass). cargo clippy -p socket-patch-core --all-targets -D warnings still fails, but none of the errors are in the file I changed.

Non-blocking, for the PR description:

The lock-importer reader merge (formats/pnpm/workspace.rs, member_importers) also changes behavior. Comments are now skipped, a column-0 # no longer ends importers:, and tab indents count as indented.
The walk bound now counts the union of all globs against MAX_MANIFESTS.

I removed Ready for review until CI and Bugbot pass on the new head.

bugbot run

Generated by Claude Code

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Mikola Lysenko (mikolalysenko) commented Oct 30, 2026

Copy link

Copy Markdown

Collaborator Author

BugBot review

Generated by Claude Code

cursor[bot] reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

cursor Bot reviewed

Oct 30, 2026

View reviewed changes

cursor Bot left a comment •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Mikola Lysenko (mikolalysenko) added the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label

Oct 30, 2026

Mikola Lysenko (mikolalysenko) commented Oct 30, 2026

Copy link

Copy Markdown

Collaborator Author

Burn-down agent: labeled Ready for review at a7ee3934.

CI: 31/31 run checks green (19 skipped by path filters); mergeable, no conflicts.
Bugbot: reviewed a7ee393 with no new issues; the one earlier thread is resolved.
Reviewer note: a7ee393 (from the final review) makes modeled() refuse three brace shapes pnpm lists differently; worth a look.
Slack announcement could not be sent from this run; the next run will retry.

Generated by Claude Code

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Mikola Lysenko (mikolalysenko) mentioned this pull request

Oct 30, 2026

CI performance dashboard #1182

Open


          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) <[email protected]>
Claude-Session: https://claude.ai/code/session_011f9aeRR24V93EPLyuXR8tp

Mikola Lysenko (mikolalysenko) removed the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label

Oct 30, 2026

Mikola Lysenko (mikolalysenko) commented Oct 30, 2026

Copy link

Copy Markdown

Collaborator Author

Final review of a7ee393. I found one blocking problem and pushed a fix, 2161606.

The problem: an untrusted pnpm-workspace.yaml could stall the scan for minutes. member_dirs walks the tree once for each brace alternative. The MAX_MANIFESTS bound only counts directories that match, so an alternative that matches nothing walks the whole tree and never reaches the bound. modeled() accepts up to 4095 alternatives. For example, **/{a..z}{a..z}{a..e} expands to 3380 of them, and on a DiskTree of about 4,200 directories it took 519 s, against 262 ms for **/zz. A scan reaches this path whenever the scanned repo sets sharedWorkspaceLockfile: false and has no shared root lock. Before this PR, every brace glob was refused, so this exposure is new.

The fix: member_dirs now walks at most 64 brace alternatives across all globs and answers Err beyond that. Callers already treat Err as no answer. The member_dirs_refuse_a_glob_the_bounded_walk_cannot_finish test now also covers this case.

Verified:

cargo test -p socket-patch-core --lib -- pnpm member_importers governing_root: 501/501 pass.
cargo clippy -p socket-patch-core -- -D warnings: clean.
rustfmt: clean.
With the old modeled() refusal put back, 5 of the new tests fail, so the tests catch the change.

Non-blocking, for the description: two changes go beyond the brace/class fix, and the description doesn't mention them. Both are tested and look right.

The lock importers: reader now skips # comments.
root_lock_lists_members is merged into member_importers. As a side effect, a tab-indented top-level key no longer ends the section.

I removed Ready for review. The burn-down agent will re-label it once CI is green, and the final-review brief will follow.

bugbot run

Generated by Claude Code

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Mikola Lysenko (mikolalysenko) commented Oct 30, 2026

Copy link

Copy Markdown

Collaborator Author

BugBot review

Generated by Claude Code

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

cursor Bot reviewed

Oct 30, 2026

View reviewed changes

cursor Bot left a comment •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

crates/socket-patch-core/src/utils/pnpm_workspace.rs Resolved

Uh oh!

There was an error while loading. Please reload this page.


          Refuse pnpm brace ranges across punctuation

5ca5e0e

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

Mikola Lysenko (mikolalysenko) commented Oct 30, 2026

Copy link

Copy Markdown

Collaborator Author

[agent] Fixed the final review's open question in 5ca5e0e instead of leaving it as a follow-up. This PR introduced it, and {-..0} lists packages itself, which is a wrong member set. A {x..y} sequence now needs both ends to be digits, both lowercase or both uppercase (same_kind_range). packages/{-..0} and packages/{0..Z} are in the refused-syntax test and fail on d52f3b6. Locally, 90 targeted core tests pass and clippy is clean.

Generated by Claude Code

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Mikola Lysenko (mikolalysenko) commented Oct 30, 2026

Copy link

Copy Markdown

Collaborator Author

BugBot review

Generated by Claude Code

cursor[bot] reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

cursor Bot reviewed

Oct 30, 2026

View reviewed changes

cursor Bot left a comment •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Mikola Lysenko (mikolalysenko) disabled auto-merge

October 30, 2026 07:51


          Refuse pnpm brace sets pnpm reads as sequences or class members

032b960

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) <[email protected]>
Claude-Session: https://claude.ai/code/session_011f9aeRocMg7S8GHB43bJsX

Mikola Lysenko (mikolalysenko) removed the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label

Oct 30, 2026

Mikola Lysenko (mikolalysenko) commented Oct 30, 2026

Copy link

Copy Markdown

Collaborator Author

Final review found two globs where member_dirs listed a different member set than pnpm 10.28.0 does, where the PR's rule is to refuse rather than guess. I fixed both in 032b960:

A .. beside a comma (packages/{a,b..c}, {a,..}, {a,b..c,x}). pnpm expands it as a sequence and also lists , 0 A [ _ and others. We listed packages/a plus a literal packages/b..c. Such sets are now refused. Only a lone same-kind sequence like {a..c} is read.
Braces inside a class (packages/[a{b,c}]). pnpm reads the braces as class members and also lists dirs named , { }. We listed only a, b, c. Classes are now checked on the raw glob as well as on each brace alternative, so {[ab],x} is still read.

All four globs are added to the refusal table. Local results: cargo test -p socket-patch-core --lib -- pnpm_workspace workspace_globs governing_root formats::pnpm::workspace passes 90/90, and cargo clippy --locked -p socket-patch-core --all-features -D warnings is clean.

I removed Ready for review; the burn-down re-labels the PR once CI is green. A brief follows on the next pass.

bugbot run

Generated by Claude Code

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Mikola Lysenko (mikolalysenko) commented Oct 30, 2026

Copy link

Copy Markdown

Collaborator Author

BugBot review

Generated by Claude Code

cursor[bot] reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

cursor Bot reviewed

Oct 30, 2026

View reviewed changes

cursor Bot left a comment •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Mikola Lysenko (mikolalysenko) added the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label

Nov 1, 2026

Mikola Lysenko (mikolalysenko) commented Nov 1, 2026

Copy link

Copy Markdown

Collaborator Author

Ready for review (burn-down agent) at 032b960.

CI: 31/31 run checks green (19 skipped). Mergeable, no conflicts.
Bugbot: reviewed 032b960 (the final-review fix commit), no findings; both earlier threads are resolved.
Reviewer note: brace sets that pnpm reads as sequences or as class members are now refused, not guessed. See the 09:39 UTC comment from 2026-10-30.

Generated by Claude Code

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Mikola Lysenko (mikolalysenko) commented Nov 1, 2026

Copy link

Copy Markdown

Collaborator Author

Final review brief (replaces the brief for `d52f3b6`)

What it does: pnpm packages: globs with brace sets ({a,b}, {a..c}) and plain character classes ([ab], [a-z]) are now read by the shared workspace_globs matcher instead of being refused outside hosted mode. modeled() now refuses only the shapes that pnpm reads differently. Hosted mode's second reader is deleted, and the two lock-importer detectors are merged into one member_importers, which now skips # comments. The two commits since the last brief refuse brace ranges across punctuation, and brace sets that pnpm reads as sequences or as class members.

Risk: low to medium. Glob semantics are subtle, and pnpm's behavior was probed on 10.28 only. Every gap fails closed: a refusal becomes Unresolved, false or "governed". Brace walks are capped at 64 alternatives and expansion at 4096. The tests were red before the fix and cover the disk tree, the in-memory tree and 26 refusal shapes.

Look here

pnpm_workspace.rs:311: modeled, with the per-alternative class check at L344
pnpm_workspace.rs:369: plain_braces, plus the comma/.. and same_kind_range rules from the new commits
pnpm_workspace.rs:484: the walk budget and the MAX_MANIFESTS union check
formats/pnpm/workspace.rs:167: the merged member_importers, which now skips column-0 comments
governing_root.rs:666: unwrap_or(false) replaces the deleted fallback

Verified: I read the full diff and traced expand_braces, char_class, expand_glob_bounded and wildcard_match against the new code. Walking the widened superset is sound, and its dot-hiding agrees with glob_matches_no_dot. There is no debug code, and CHANGELOG.md is untouched. CI is 50/50 green, and Bugbot passed.

Changes I made: none.

Open questions (non-blocking):

Hosted behavior change: for globs it still refuses (e.g. [!a]*, {1..10}), hosted mode now lists nothing instead of guessing. This follows the "errs toward not a member" rule, but it is visible to users.
Different refusal sets: lists_member doesn't share member_dirs's 64-alternative cap. A glob with 65 to 4095 alternatives resolves in hosted mode but gives Unresolved in workspace_members. Both outcomes fail closed.

Auto-merge is armed, so approving sends this straight to the merge queue.

Generated by Claude Code

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Mikola Lysenko (mikolalysenko) enabled auto-merge

November 1, 2026 00:54

Mikola Lysenko (mikolalysenko) mentioned this pull request

Nov 1, 2026

Sweep core constants.rs and utils (concurrent, durability, …) #1907

Merged

Wenxin Jiang (Wenxin-Jiang) approved these changes

Nov 2, 2026

View reviewed changes

Wenxin Jiang (Wenxin-Jiang) left a comment

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[agent] Reviewed 032b9603af30ace16dd89c6bc8d8c977e9786ce8. Checked bounded brace/class workspace expansion, membership/importer agreement, range syntax and large plain workspace sets; existing inline findings are addressed at this head. No major correctness issues found. Validation: source and regression-test review plus the available exact-head CI checks; no local test pass is claimed.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Mikola Lysenko (mikolalysenko) added this pull request to the merge queue

Nov 2, 2026

github-merge-queue Bot removed this pull request from the merge queue due to a conflict with the base branch

Nov 2, 2026

Mikola Lysenko (mikolalysenko) removed the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label

Nov 2, 2026


          Merge main into agent/fix-pnpm-brace-class-globs

6dc33e2

main (#1952) already merged the two pnpm lock-importer detectors this
PR also merged. Keep main's generic lock_has_member_importers and
lock_text_has_member_importers, and drop this PR's duplicate copy and
its overlapping tests. The brace and class glob changes are unchanged.

Assisted-by: Claude Code:claude-opus-5-5

Mikola Lysenko (mikolalysenko) commented Nov 2, 2026

Copy link

Copy Markdown

Collaborator Author

BugBot review

Generated by Claude Code

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

cursor Bot reviewed

Nov 2, 2026

View reviewed changes

cursor Bot left a comment •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

crates/socket-patch-core/src/utils/pnpm_workspace.rs Resolved

Uh oh!

There was an error while loading. Please reload this page.


          Test pnpm brace groups without a comma stay literal

8a311af

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

Mikola Lysenko (mikolalysenko) commented Nov 2, 2026

Copy link

Copy Markdown

Collaborator Author

BugBot review

Generated by Claude Code

cursor[bot] reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

cursor Bot reviewed

Nov 2, 2026

View reviewed changes

cursor Bot left a comment

Copy link

Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit 8a311af. Configure here.}

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Mikola Lysenko (mikolalysenko) added this pull request to the merge queue

Nov 2, 2026

Merged via the queue into main with commit f54766b

Nov 2, 2026

50 checks passed

Uh oh!

There was an error while loading. Please reload this page.

Mikola Lysenko (mikolalysenko) deleted the agent/fix-pnpm-brace-class-globs branch

November 2, 2026 18:56

Mikola Lysenko (mikolalysenko) commented Nov 2, 2026

Copy link

Copy Markdown

Collaborator Author

[final reviewer] Re-enqueued: approved, ci-ok and clippy green on the current head, mergeable, and no unresolved threads. The earlier eviction made GitHub turn off auto-merge, so I armed it again (squash) and the PR went straight back into the merge queue. The approval still counts after the post-approval commits.

Generated by Claude Code

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Reviewers

cursor[bot]

cursor[bot] left review comments

Wenxin Jiang (Wenxin-Jiang)

Wenxin-Jiang approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

pnpm workspace membership refuses brace and class globs the shared matcher already reads, and hosted keeps a second reader for them

Uh oh!

There was an error while loading. Please reload this page.

3 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

Conversation

Mikola Lysenko (mikolalysenko) commented Oct 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

LLM Description written by Claude Code:claude-opus-5-5

Summary

Root cause

Changes

Test evidence

/code-review high

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 29, 2026

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 29, 2026

Uh oh!

cursor Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 29, 2026

Uh oh!

cursor Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 30, 2026

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 30, 2026

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 30, 2026

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 30, 2026

Uh oh!

cursor Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 30, 2026

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 30, 2026

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 30, 2026

Uh oh!

cursor Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 30, 2026

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 30, 2026

Uh oh!

cursor Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 30, 2026

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 30, 2026

Uh oh!

cursor Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mikola Lysenko (mikolalysenko) commented Nov 1, 2026

Uh oh!

Mikola Lysenko (mikolalysenko) commented Nov 1, 2026

Final review brief (replaces the brief for d52f3b6)

Uh oh!

Wenxin Jiang (Wenxin-Jiang) left a comment

Choose a reason for hiding this comment

Uh oh!

Mikola Lysenko (mikolalysenko) commented Nov 2, 2026

Uh oh!

cursor Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 29, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

cursor Bot left a comment •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

cursor Bot left a comment •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

cursor Bot left a comment •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

cursor Bot left a comment •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

cursor Bot left a comment •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

cursor Bot left a comment •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Final review brief (replaces the brief for `d52f3b6`)

cursor Bot left a comment •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.