fix(glob): normalize pattern before brace expansion#311
Conversation
🦋 Changeset detectedLatest commit: 25be777 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
commit: |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7d59c84c0c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| // the caller sees an obvious zero-match outcome rather than a silent | ||
| // partial walk. | ||
| const subPatterns = expandBraces(args.pattern); | ||
| const subPatterns = expandBraces(normalize(args.pattern)); |
There was a problem hiding this comment.
For patterns that rely on kaos' escape syntax to match literal glob characters, e.g. foo/\*.ts for a file literally named *.ts or \{a,b\}.ts for literal braces, normalizing the whole pattern first rewrites those backslashes as path separators before expandBraces and globPatternToRegex can treat them as escapes. That turns literal-match requests into wildcard matches or unintended brace expansions, so the Glob tool can return the wrong files whenever users escape glob metacharacters.
Useful? React with 👍 / 👎.
| // the caller sees an obvious zero-match outcome rather than a silent | ||
| // partial walk. | ||
| const subPatterns = expandBraces(args.pattern); | ||
| const subPatterns = expandBraces(normalize(args.pattern)); |
There was a problem hiding this comment.
When a brace alternative itself contains path segments with .., normalizing before expandBraces can collapse across the still-unparsed brace text and corrupt the pattern. For example, src/{foo/../bar,baz}/*.ts should expand to the normalized alternatives src/bar/*.ts and src/baz/*.ts, but POSIX-style normalization first turns it into src/bar,baz}/*.ts, leaving no valid brace group for the supported brace-expansion path and causing missed matches for this otherwise valid pattern shape.
Useful? React with 👍 / 👎.
| // the caller sees an obvious zero-match outcome rather than a silent | ||
| // partial walk. | ||
| const subPatterns = expandBraces(args.pattern); | ||
| const subPatterns = expandBraces(normalize(args.pattern)); |
There was a problem hiding this comment.
When a glob pattern contains a wildcard component before .., path normalization treats the wildcard text as an ordinary directory name and rewrites the pattern to a different search scope before kaos sees it. For example, src/**/foo/../*.ts becomes src/**/*.ts, so a query that was constrained around foo can now match every TypeScript file under src; similarly src/*/../*.ts becomes src/*.ts. This only affects patterns with .. adjacent to wildcard segments, but in those cases Glob can return unrelated matches.
Useful? React with 👍 / 👎.
…aces correctly - expand braces before normalizing to prevent `..` from collapsing across brace groups - skip normalization for patterns containing backslash-escaped glob metacharacters - add test coverage for escaped braces and nested `..` inside alternatives
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 25be777730
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| // partial walk. | ||
| const subPatterns = expandBraces(args.pattern); | ||
| const subPatterns = expandBraces(args.pattern).map((p) => | ||
| hasGlobEscape(p) ? p : normalize(p), |
There was a problem hiding this comment.
For Windows-style glob patterns where a backslash separator is immediately before a wildcard, such as src\*.ts or src\**\*.ts, this branch treats the separator as an escaped glob metacharacter and skips normalization. kaos.glob only splits patterns on / (packages/kaos/src/local.ts:252), and its regex builder then interprets \* as a literal *, so these inputs search the root for a literal src*.ts-style name instead of walking under src. This leaves the Windows-backslash normalization path broken for the most common wildcard cases.
Useful? React with 👍 / 👎.
1 participant
Summary
Normalize glob patterns before brace expansion to prevent incorrect path matching when patterns contain redundant path segments (e.g.
foo//bar,foo/./bar) or Windows backslashes. Thenormalizecall was already present in the codebase but was applied too late — afterexpandBraceshad already split the pattern, so individual sub-patterns were never normalized.1. Brace expansion order fix
Problem:
expandBracesfunction split the unnormalized pattern first.kaos.globwithout normalization, causing mismatches on backends where path separators matter.What was done:
normalize(args.pattern)to happen beforeexpandBraces(...)instead of after.Checklist
gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update.