Skip to content

fix(frontmatter): expand nested brace globs in paths: correctly#1701

Open
adityachaudhary99 wants to merge 2 commits into
Gitlawb:mainfrom
adityachaudhary99:fix/frontmatter-nested-brace-expansion
Open

fix(frontmatter): expand nested brace globs in paths: correctly#1701
adityachaudhary99 wants to merge 2 commits into
Gitlawb:mainfrom
adityachaudhary99:fix/frontmatter-nested-brace-expansion

Conversation

@adityachaudhary99

@adityachaudhary99 adityachaudhary99 commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

What & why

expandBraces used the regex ^([^{]*)\{([^}]+)\}(.*)$, whose [^}]+ stops at the first }, so nested brace groups in a paths: frontmatter glob were corrupted: {a,{b,c}} became ["a}","b","c}"] and src/**/*.{js,{ts,tsx}} produced stray } and broken globs — silently breaking path-scoped skill / CLAUDE.md activation.

Changes

Replaced the regex extraction with a depth-aware scan that finds the matching close brace and splits on top-level commas only, recursing as before. Unbalanced braces fall back to the literal input.

Checks

Added src/utils/frontmatterParser.test.ts covering no-braces, single group, nested groups, nested globs, sibling groups, and the unbalanced-brace fallback.

(Small self-contained bug fix with no existing issue.)

Summary by CodeRabbit

  • Bug Fixes

    • Improved frontmatter path brace expansion to correctly handle nested brace groups, multiple sibling groups (cartesian-product expansion), unbalanced brace patterns (left unchanged), and literal {} preservation for accurate glob generation.
  • Tests

    • Added a Bun test suite covering brace-expansion behavior, including nested groups, edge cases (escaped and unbalanced inputs), and scenarios that previously risked producing invalid or stray brace output.

expandBraces used the regex `^([^{]*)\{([^}]+)\}(.*)$`, whose `[^}]+` stops
at the first `}`, so nested brace groups were corrupted: `{a,{b,c}}` became
`["a}","b","c}"]` and `src/**/*.{js,{ts,tsx}}` produced stray `}` and broken
globs — silently breaking path-scoped skill / CLAUDE.md activation. Replace
the regex with a depth-aware scan that finds the matching close brace and
splits on top-level commas only, recursing as before. Unbalanced braces fall
back to the literal input. Adds frontmatterParser.test.ts.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 17, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 64834c16-deac-4224-8bc7-00dc5466b471

📥 Commits

Reviewing files that changed from the base of the PR and between 2739aeb and 7b5f736.

📒 Files selected for processing (2)
  • src/utils/frontmatterParser.test.ts
  • src/utils/frontmatterParser.ts
📜 Recent review details
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: smoke-and-tests
  • GitHub Check: typecheck
🧰 Additional context used
📓 Path-based instructions (10)
src/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Use TypeScript with strict mode and ESM imports

Files:

  • src/utils/frontmatterParser.test.ts
  • src/utils/frontmatterParser.ts
{src/services/**/*.ts,src/utils/**/*.ts}

📄 CodeRabbit inference engine (AGENTS.md)

Use execa for child processes

Files:

  • src/utils/frontmatterParser.test.ts
  • src/utils/frontmatterParser.ts
**/*.{ts,tsx,js,jsx,py,json,md}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Follow the existing code style in the touched files

Files:

  • src/utils/frontmatterParser.test.ts
  • src/utils/frontmatterParser.ts
**/*.test.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Add or update tests when the change affects behavior

Files:

  • src/utils/frontmatterParser.test.ts
**/*.test.{ts,tsx,js}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Test the exact provider/model path you changed when possible

Files:

  • src/utils/frontmatterParser.test.ts
**/*.{ts,tsx,js,jsx,py}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Keep comments useful and concise

Files:

  • src/utils/frontmatterParser.test.ts
  • src/utils/frontmatterParser.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Follow TypeScript strict mode and type safety practices by running typecheck before submitting

Files:

  • src/utils/frontmatterParser.test.ts
  • src/utils/frontmatterParser.ts
**/*

⚙️ CodeRabbit configuration file

**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.

Files:

  • src/utils/frontmatterParser.test.ts
  • src/utils/frontmatterParser.ts
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}

⚙️ CodeRabbit configuration file

{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}: Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions. Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior.

Files:

  • src/utils/frontmatterParser.test.ts
**

⚙️ CodeRabbit configuration file

**: # AGENTS.md - AI Agent Coding Guide

This guide is for AI coding agents working in the OpenClaude repository. Read it before changing code, and also follow CONTRIBUTING.md for contributor policy, PR expectations, review follow-up, and project scope.

Project Snapshot

OpenClaude is a coding-agent CLI for cloud and local model providers. It supports OpenAI-compatible APIs, Anthropic, Gemini, DeepSeek, Ollama, MCP, local backends, slash commands, tools, agents, and a React/Ink terminal UI.

The installed CLI runs on Node.js >=22.0.0. Bun is used for source builds, scripts, dependency management, and tests.

Work Style

  • Keep changes focused on one problem.
  • Prefer existing patterns in the file or nearby module.
  • Avoid unrelated formatting, renames, dependency changes, or broad rewrites.
  • Add or update tests when behavior changes.
  • Update docs when setup, commands, provider behavior, or user-facing behavior changes.
  • For new features, larger refactors, dependencies, or runtime changes, follow the issue-first guidance in CONTRIBUTING.md.

Stack And Conventions

  • TypeScript with strict mode and ESM imports.
  • React + Ink for terminal UI.
  • Bun lockfile and Bun scripts for development workflows.
  • Node runtime for the built CLI.
  • Python exists for legacy/local-provider helper code. Do not add new Python code or expand Python-based features unless a maintainer explicitly approves that direction.

Common libraries and patterns:

  • chalk for terminal color.
  • commander for CLI argument parsing.
  • execa for child processes.
  • Existing service, provider, settings, permission, and UI patterns over new abstractions.

Repository Map

  • src/commands/ - slash and CLI command implementations.
  • src/components/ - React/Ink UI components.
  • src/services/ - API, MCP, OAuth, wiki, voice, and other service integrations.
  • src/tools/ - tool implementations.
  • src/utils/ - shared utilities.
  • `src/integration...

Files:

  • src/utils/frontmatterParser.test.ts
  • src/utils/frontmatterParser.ts
🔇 Additional comments (2)
src/utils/frontmatterParser.ts (1)

286-296: LGTM!

src/utils/frontmatterParser.test.ts (1)

44-55: LGTM!


📝 Walkthrough

Walkthrough

expandBraces in frontmatterParser.ts is rewritten from a simple regex split to a depth-tracking scanner that locates matching brace pairs, splits alternatives only at depth 1, handles unbalanced and empty braces literally, and recursively processes remaining groups. A comprehensive test suite covers all expansion cases.

Changes

Balance-aware brace expansion rewrite

Layer / File(s) Summary
expandBraces algorithm rewrite
src/utils/frontmatterParser.ts
expandBraces replaces the regex approach with a depth-tracking scan: locates the matching } for the first {, splits alternatives on commas only at depth 1, returns input unchanged for unbalanced braces, treats empty {} as literal text (not expanded to empty string), and recursively expands remaining brace groups in prefix/suffix. JSDoc examples updated.
Test suite for brace expansion
src/utils/frontmatterParser.test.ts
Tests for splitPathInFrontmatter exercise the internal expandBraces logic: patterns without braces pass through unchanged, single-level and nested brace alternations expand correctly without stray braces, nested glob patterns expand into valid glob strings, sibling brace groups expand as cartesian products, unbalanced braces return input unchanged, and empty {} groups are preserved literally both standalone and when combined with later expansions.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 7
✅ Passed checks (7 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main fix: correcting nested brace glob expansion in frontmatter paths, matching the actual changes in expandBraces implementation and test coverage.
Description check ✅ Passed The description covers what changed (regex replaced with depth-aware scan), why it mattered (nested braces were corrupted), and testing (new test file added). However, it lacks the structured sections and checkboxes from the template.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Risk Surface Disclosed ✅ Passed PR touches skills/plugins (path-scoped skill activation) and explicitly discloses this in the description, stating it fixes "silently breaking path-scoped skill/CLAUDE.md activation." The fix is a...
No Hidden Policy Change ✅ Passed PR modifies only brace expansion parsing in frontmatterParser.ts/test.ts; no product, trust-model, routing, telemetry, network, or permission-policy changes detected.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot]
coderabbitai Bot previously approved these changes Jun 17, 2026

@jatmn jatmn left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I found an issue that needs to be addressed before this is ready.

Findings

  • [P2] Preserve literal empty brace groups in path frontmatter
    src/utils/frontmatterParser.ts:305
    Empty brace groups were previously not considered expandable because the old regex required at least one character inside the braces, so patterns containing a literal {} segment stayed literal. With the new scanner, paths: "{}" becomes [""], and both parseSkillPaths and the CLAUDE.md path parser filter that empty string and treat the file as having no path restriction, so a conditional skill or rule applies globally. The same collapse also retargets literal paths such as src/{}file.ts to src/file.ts, and broadens src/{} to src/, activating on the wrong files. Please keep empty brace groups literal, or otherwise avoid converting an empty brace expansion into a different or unrestricted path rule.

The balance-aware scanner expanded `{}` to a single empty alternative, so
`paths: "{}"` collapsed to [""]; parseSkillPaths and the CLAUDE.md path
parser drop that empty string and treat the file as having no path
restriction (activating everywhere). The previous regex required >=1 inner
char, so `{}` stayed literal. Restore that: treat an empty brace group as
literal while still expanding any later groups in the suffix. Adds tests.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@jatmn jatmn left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for the update. I rechecked the previously discussed paths and do not see any remaining actionable issues from my side.

@kevincodex1 LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants