glob: stop a non-segment ** from swallowing the ** segments after it in match()

[robobun]

Problem

• new Bun.Glob("a**/**").match("ab") returns true. a** means a* in Bun (documented in the matcher, covered by the a**c / foo** tests), so a**/** means a*/**, which correctly returns false for "ab" (so do picomatch and bash).
• Same for every shape where a ** that is part of a larger segment is directly followed by ** segments: x**/** matches "x", ?**/** matches "b", {x,a}**/** matches "ab", **/a**/** matches "x/ab", a**/**/b matches "ab", and !a**/** gives the inverse wrong answer. a**/b is unaffected.
• match() also disagreed with scan(): the walker splits the pattern on / and never had this problem, so a**/** yields no entry for a file ay while match("ay") says true.
• Cause, src/glob/matcher.rs, b'*' arm of glob_match_impl: as soon as two stars are seen, skip_globstars folds the following /**/ runs and a trailing /** into this **. Only afterwards does the code check whether the ** is actually at the start of a segment (preceded by /, or at glob_start). When that check fails the ** falls back to plain * behaviour, but the /** segments it was supposed to leave for the rest of the pattern are already gone, so a**/** is matched as a*. (Previously the check also looked behind the last ** of the collapsed run, which is always preceded by /, so it passed for the wrong reason.)

Fix

• Evaluate the segment-start condition (now the starts_segment helper) on the first ** of the run, as part of is_globstar, before calling skip_globstars. A ** that does not start a segment is now matched as two ordinary *s, and the /** segments after it are matched on their own, which is what a*/** does. The post-collapse check now only has to look at what follows the run.
• Correct because the collapse is only valid when the whole run is one globstar; the condition is unchanged (glob_index <= glob_start || glob[glob_index - 1] == '/' is exactly what the old glob_index + 2 - glob_start < 3 || glob[glob_index + 2 - 3] == '/' computed on an uncollapsed **), it is just evaluated before the collapse instead of after it. For a ** that does start a segment nothing changes: every ** reached by the collapse is preceded by /, so the old check was equivalent for them. The <= is still needed because a failing brace branch backtracks to a ** that precedes the branch (**/{a,b}), where glob_index is below the branch's glob_start; the test pins that case.
• Relationship to glob: judge a ** by its own surroundings, not by the brace branch being matched #39004: that PR fixes a second, separable bug in the same rule (after backtracking out of a brace branch, glob_start is the wrong reference point) by replacing the glob_start comparison with a lookup in the brace stack, and it is written on top of this change. Either order works: if this lands first, glob: judge a ** by its own surroundings, not by the brace branch being matched #39004 rebases to the rule swap plus its own tests; if glob: judge a ** by its own surroundings, not by the brace branch being matched #39004 lands first, this PR is redundant and can be closed.
• Test: test/js/bun/glob/match.test.ts, "** inside a segment stays a plain * when ** segments follow it". 11 of its assertions plus each row of the a**/** vs a*/** table fail on the current release; the controls (with the / present, collapsed globstar runs, brace backtracking) pass before and after.
• All 30 tests in match.test.ts (1569 assertions, including the ported bash/micromatch star and globstar suites) pass with the debug build.
• Differential check against the previous build: 8612 patterns over an 8-token grammar x 128 paths (1,102,336 results). 438 results across 22 patterns changed, every one of them a pattern with a non-segment ** followed by ** segments; within that class agreement with picomatch 4 went from 2884/3328 to 3320/3328, and the 8 remaining are !pattern against "", where picomatch rejects empty input regardless of the pattern while Bun returns the complement of the un-negated pattern, as its existing negation tests require.
• scan() is unchanged: it matches each component separately (a** and ** are separate components), so it was already right and now agrees with match().
• Not changed here: ** at the end of a brace branch (a/{**,b}) is still treated as *, and re-scanning a pattern prefix from inside a brace branch still compares against the branch's glob_start. Both are pre-existing and independent of this fix.

Background

• The matcher is a backtracking matcher over the raw pattern bytes (ported from the glob-match crate); it does not expand braces or pre-tokenize. Each * records a wildcard resume point; ** that qualifies as a globstar additionally records a globstar resume point that is allowed to cross /.
• "Globstar" here means a ** that is a whole path segment: preceded by / (or the start of the pattern / of the brace branch being tried, which is what glob_start is) and followed by / (or the end of the pattern). Any other **, such as a** or **b, is defined to behave like *; see the a**c, foo**bar and foo** cases in match.test.ts.
• skip_globstars is the optimization that turns **/**/** into a single globstar so the matcher does not have to backtrack through equivalent segments. It is only meaningful when the first ** of the run is itself a globstar, which is the invariant this change restores.
• glob_start is the index at which the current glob_match_impl invocation began: the pattern start (after any !) at top level, or the branch start when matching a brace alternative, so that {**/a,**/b} treats both ** as segment starts.

[coderabbitai]

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 6 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: c25aa99a-da41-4b30-8ee8-8c4b539d382a

📥 Commits

Reviewing files that changed from the base of the PR and between 732491c and 3751b56.

📒 Files selected for processing (2)

• src/glob/matcher.rs
• test/js/bun/glob/match.test.ts

---

_{Comment @coderabbitai help to get the list of available commands.}

[robobun]

^{Updated 7:39 AM PT - Aug 15th, 2026}

@robobun, your commit 3751b56 is building: #98186

[robobun]

Status: closed, superseded by #39004, which carries the same matcher change plus the fix for the related brace-branch bug and covers this shape in its tests.

Reproduced on bun 1.4.0 (release) with new Bun.Glob("a**/**").match("ab") returning true while new Bun.Glob("a*/**").match("ab") returns false; the test in this branch fails there and passes with the fix.

[claude]

LGTM — focused fix that moves the segment-start check before skip_globstars so a non-segment ** no longer swallows the /** segments after it.

What was reviewed:

• Traced a**/** through both old and new is_globstar paths; confirmed the old post-collapse glob[glob_index-3] check saw the / before the last collapsed ** and passed incorrectly, and the new pre-collapse check correctly rejects it.
• Verified <= in glob_index <= glob_start short-circuits before glob[glob_index-1] for the brace-backtrack case (**/{a,b} → glob_index=0, glob_start=4), matching the old saturating_sub behavior with no underflow.
• Checked that segment-start globstars (**, !**, {**/a,...}, a/**) and the **b trailing case are unchanged; the acknowledged pre-existing brace-prefix-rescan quirk is preserved, not regressed.
• Test adds direct assertions, a 17-path × 11-pattern equivalence table against the a*/** spelling, and controls pinning collapse and brace-backtrack behavior.

Extended reasoning...

Overview

Two files: a ~10-line logic change in src/glob/matcher.rs (the b'*' arm of glob_match_impl) and a new test block in test/js/bun/glob/match.test.ts. The fix hoists the "preceded by / or at glob_start" condition from the post-skip_globstars check into the is_globstar computation itself, so skip_globstars never runs on a ** that isn't at a segment boundary. The now-redundant "preceded by" clause is dropped from the later check, which retains only the "followed by" half.

Correctness trace

For a**/** at glob_start=0: old code set is_globstar=true (two stars), ran skip_globstars (glob_index → 4, pointing at the second **), then += 2 → 6, then checked glob[6-3]='/' — the / before the collapsed **, not the original one — so it wrongly entered the globstar path with the trailing /** already consumed. New code checks glob[1-1]='a' != '/' before collapsing, so is_globstar=false; the two *s advance one each, and the /** at index 4 is later matched as its own globstar (glob[3]='/'). This is exactly a*/**.

For real globstars nothing changes: at glob_start the <= clause fires; after a / the glob[idx-1]=='/' clause fires; skip_globstars only ever advances over /** runs, so every ** it lands on is preceded by /, meaning the old post-check was equivalent to the new pre-check for the valid case. The <= (vs ==) preserves the old saturating_sub semantics for the brace-backtrack case where glob_index < glob_start, and short-circuits before the glob[idx-1] read so there's no underflow at index 0.

Security risks

None. Pure byte-slice matching with no allocation, no I/O, no unsafe. All indexing is bounds-guarded (glob_index+1 < glob.len() before glob[idx+1]; <= short-circuits before glob[idx-1]). Input is not trusted for anything beyond the match result.

Level of scrutiny

Medium. Bun.Glob is user-facing and used by the bundler/resolver, but this is a narrow correctness fix in a pure function with a large existing test suite (1569 assertions in the file, including ported bash/micromatch suites) that still passes, plus a 1.1M-result differential showing only the targeted pattern class changed. The change is mechanically small and the reasoning in the PR description is precise and checks out against the code.

Other factors

Test quality is high: direct falsy/truthy assertions for the reported shapes, an equivalence table asserting each a**/**-form pattern matches identically to its a*/** spelling across 17 paths (so a regression in either direction fails), and control cases pinning that legitimate globstar collapse (a/**/**/b) and brace backtracking (**/{a,b}) are unchanged. The PR explicitly scopes out two pre-existing brace-related quirks and preserves their behavior. No prior reviews to address; CI is building.

[robobun]

Heads-up on overlap: #39004 fixes a neighbouring bug in the same arm (**/a**/{x,y} matching ab/ay, which is the glob_index <= glob_start condition misfiring after a brace branch backtracks into an enclosing globstar). It replaces that condition with a check of the **'s own surroundings, evaluated before skip_globstars as here, so it also covers the a**/** shape from this PR. Whichever of the two lands second will need a small rebase; the tests in both PRs remain valid either way.

[github-actions]

If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code

[github-actions]

If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code

[claude]

I reviewed this PR and didn't find any bugs — the fix is sound (moving the segment-start check into is_globstar before skip_globstars runs is the right place for it, and the equivalence argument for the unchanged cases checks out). Deferring to a human reviewer because of the stated overlap with #39004 in the same arm — someone should decide which lands first — and because comment-cop has two open flags on the new comments that haven't been addressed.

What was reviewed:

• Traced the b'*' arm before/after: for a segment-start ** the new pre-collapse check is equivalent to the old post-collapse glob_index - glob_start < 3 || glob[glob_index-3]=='/'; for a**/** it now correctly falls through to two plain *s and leaves /** for the next iteration.
• Checked index safety on glob[glob_index - 1] — only reached when glob_index > glob_start ≥ 0, and glob_index < glob.len() from the enclosing guard.
• Verified the <= (vs <) still handles brace-branch backtracking to a ** before glob_start, matching the old saturating_sub behaviour; the **/{a,b} test pins it.
• Confirmed a**, **b, bare **, and **/** collapse are behaviorally unchanged.

Extended reasoning...

Overview

This PR changes the b'*' arm of glob_match_impl in src/glob/matcher.rs. Previously, seeing ** immediately called skip_globstars to fold trailing /** runs, then checked whether the (now-collapsed) run was preceded by /. That check looked at glob_index - 3 after the collapse, which for a**/** points at the / before the last ** in the run — always / — so the whole thing was wrongly promoted to a globstar and a**/** matched ab. The fix folds the "preceded by / or at glob_start" test into is_globstar itself, evaluated on the first ** before any collapse. When that fails, is_globstar is false, skip_globstars never runs, and each * is consumed as a plain wildcard, leaving /** intact for the rest of the pattern. The post-collapse check is simplified to only look at what follows (/ or end-of-pattern), since the leading condition is now decided upstream. A 65-line test block is added covering the failing shapes, positive controls, an equivalence table between a**/** and a*/** spellings, and the brace-backtracking case that motivates <= over <.

Security risks

None. This is pure pattern-matching logic over byte slices with no I/O, no allocation, and no external input beyond what Bun.Glob already accepts. The change tightens matching (fewer paths match), so it cannot introduce a path-traversal-style over-match; the only risk direction would be breaking a caller that relied on the buggy over-match, which the 1.1M-case differential run in the description scopes narrowly.

Level of scrutiny

Medium-high. The glob matcher is a hand-written backtracking state machine where off-by-one errors in index arithmetic are easy and silent. I traced the new condition against the old for each shape class (segment-start **, non-segment ** with and without trailing /**, bare **, **b, a**, brace-branch backtrack) and they agree everywhere except the intended bug class. Index bounds on glob[glob_index - 1] are safe. The test coverage is strong — direct assertions, an equivalence table, and controls that pin unchanged behaviour.

Other factors

Two things push this to a human:

• Overlap with #39004: the author notes that PR touches the same condition (replacing glob_index <= glob_start with a different formulation) and whichever lands second needs a rebase. A maintainer should sequence these.
• Outstanding comment-cop flags: the bot flagged both the doc-comment update (line 137, likely a false positive — it's API documentation) and the new 8-line inline comment at line 210. REVIEW.md says "Only comment what the code cannot say. One line." The 210 comment is genuinely explanatory (why <=, why before skip_globstars), but at 8 lines it's over the repo's stated bar and the flag hasn't been addressed. Per the approval guidelines I shouldn't approve over outstanding reviewer comments.

The one CI failure (test/bake/deinitialization.test.ts segfault on Windows x64) is unrelated to glob code.

[github-actions]

If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code

[claude]

LGTM — the segment-start check is the same predicate as before, just evaluated on the first ** of the run before skip_globstars collapses trailing /** segments, which is the invariant the collapse depends on.

What was reviewed:

• Traced a**/** vs ab, a**/** vs ab/c, a**b, **b, a/** through both old and new code paths — behaviour only diverges for the target class (non-segment ** followed by /**).
• Verified starts_segment(glob, glob_start, glob_index) is arithmetically identical to the old glob_index+2 - glob_start < 3 || glob[glob_index-1] == '/' on an uncollapsed **, and that dropping the post-collapse start check is safe because every collapsed /** is /-preceded by construction.
• Checked the <= (vs <) preserves the brace-backtrack case (**/{a,b} re-examined with glob_index < glob_start), pinned by the last three assertions; confirmed no underflow at glob_index - 1.
• The comment-cop linter flag on line 674 is on a two-line /// doc comment for the extracted helper — looks like a false positive after 3751b56 shortened it.

Extended reasoning...

Overview

Two-file change: src/glob/matcher.rs moves the "does this ** start a path segment" check into is_globstar (before skip_globstars runs) and extracts it as starts_segment(); the now-redundant post-collapse start check is dropped from the in_globstar conditional. test/js/bun/glob/match.test.ts gains one test block with ~30 direct assertions plus an 11×17 equivalence table asserting each a**/**-shaped pattern matches identically to its a*/** spelling.

Security risks

None. Pure boolean-matching logic over already-validated byte slices; no allocation, I/O, or external input handling changes. The only indexing added (glob[glob_index - 1]) is guarded by glob_index > glob_start ≥ 0.

Level of scrutiny

Medium. The glob matcher is user-facing and shared by Bun.Glob, the bundler, and the shell, so a regression would be visible — but the change is a reordering of an existing predicate, not new semantics. I hand-traced the state machine on the boundary shapes (a**, **b, a/**, a**/**, **/{a,b} backtrack) and confirmed the only paths where old and new diverge are exactly the bug class: a ** that isn't segment-initial but is followed by /** runs. The PR description's 1.1M-case differential against picomatch corroborates this.

Other factors

• The old predicate glob_index.saturating_sub(glob_start) < 3 || glob[glob_index-3] == '/' (evaluated at original_index + 2 when uncollapsed) reduces algebraically to original_index <= glob_start || glob[original_index-1] == '/', which is exactly starts_segment. So for every ** that already started a segment, nothing changes; for those that don't, skip_globstars is now correctly skipped and the trailing /** stays in the pattern to be matched on its own.
• The known adjacent issue (glob_index <= glob_start over-accepting when a brace branch backtracks past a non-segment **, e.g. a**/{x,y}) is pre-existing, called out in the description, and tracked by #39004 — this PR neither introduces nor worsens it.
• Test quality is high: negative cases, positive controls that pass before and after, the brace-backtrack <= case, and a table-driven equivalence check against the canonical * spelling. All 1569 existing assertions in the file reportedly still pass.
• The three comment-cop bot comments were addressed in 3751b56 by extracting the named starts_segment helper; the remaining flag on line 674 targets a standard two-line Rust doc comment and reads as a linter false positive rather than an unaddressed concern.

[robobun]

Closing as superseded by #39004.

That PR makes the same change this one does (the segment-start test moves ahead of skip_globstars, so a ** inside a segment no longer takes the /** segments after it with it) and additionally replaces the glob_index <= glob_start rule with one that does not depend on which brace branch is being matched, which fixes a second bug in the same line. Its tests cover the x**/** shape; the extra spellings and the differential evidence from this PR are summarised in a comment there.

One correction to the description above, for anyone reading this later: on the old code the a**/** run was not demoted to a*. Because the look-behind ran after the fold it inspected the byte before the last ** of the run, which is always a /, so the whole run was accepted as a globstar glued to a (a**/** matched every path starting with a, including ab/c; a**/**/b matched ab and ab/x/y/b). The symptom and the fix are as described; only the sentence about falling back to * was wrong.