feat(rules): add require_skills with timing control to rules #1396
Conversation
|
|
Summary of ChangesHello @Quentor1ne, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the agent's ability to adhere to project-specific guidelines by introducing a robust system for dynamic rule and skill enforcement. It allows for fine-grained control over when and where agents apply specific skills, making their autonomous development more precise and context-aware. Additionally, it addresses a critical macOS build stability issue, improving the overall developer experience. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
🎉 Thanks for your first PR!
A maintainer will review it soon. Please make sure:
- Your branch is synced with
develop - CI checks pass
- You've followed our contribution guide
Welcome to the Auto Claude community!
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdds Claude Code Rules support: backend rule discovery, parsing, glob matching, and aggregation from Changes
Sequence Diagram(s)sequenceDiagram
participant Frontend
participant Backend as create_client()
participant Rules as RulesLoader
participant FS as ".claude/rules/"
Frontend->>Backend: request client (project, model, agent_type, ...)
Backend->>Backend: should_use_claude_rules()
alt rules enabled
Backend->>Rules: load_claude_rules(project_dir, files_to_check)
Rules->>FS: discover rule files
FS-->>Rules: return rule files
Rules->>Rules: parse frontmatter & match globs (files_to_check or all)
Rules-->>Backend: (combined_rules_content, matched_rule_names, required_skills)
Backend->>Backend: filter skills by agent_type & phase
Backend->>Backend: append rules + skills to system prompt
Backend-->>Frontend: return ClaudeSDKClient (prompt contains rule content)
else rules disabled
Backend-->>Frontend: return ClaudeSDKClient (base prompt)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
The pull request introduces support for Claude Code rules, allowing for path-based rule application and timing control for required skills. This is a significant feature enhancement that improves the agent's ability to adhere to project-specific guidelines. The changes also include a macOS build fix related to file copying. The documentation for the new feature is comprehensive and well-structured.
|
|
||
| if [ -d "$SRC_DIR" ]; then | ||
| echo "[prebuild-mac] Pre-copying site-packages with ditto..." | ||
| rm -rf "$FRONTEND_DIR/dist" |
There was a problem hiding this comment.
The command rm -rf "$FRONTEND_DIR/dist" is very aggressive as it deletes the entire dist directory. While this might be intended to ensure a clean state, it could potentially remove other build artifacts or cause issues if subsequent build steps expect dist to be partially populated. It would be safer to only remove the specific python-site-packages directory within dist to avoid unintended side effects.
| rm -rf "$FRONTEND_DIR/dist" | |
| rm -rf "$DEST_DIR" |
apps/backend/core/client.py
Outdated
| import re | ||
| match = re.search(r'\[(.*)\]', stripped) |
There was a problem hiding this comment.
The import re statements are placed inside the _parse_rule_frontmatter function. Importing modules inside a function can lead to performance overhead if the function is called frequently. It's generally better practice to place imports at the top of the file.
import re
match = re.search(r'\[(.*)\]', stripped)
apps/backend/core/client.py
Outdated
| if "**" in pattern: | ||
| # Split pattern into parts | ||
| parts = pattern.split("**") | ||
| if len(parts) == 2: | ||
| prefix, suffix = parts | ||
| prefix = prefix.rstrip("/") | ||
| suffix = suffix.lstrip("/") | ||
|
|
||
| # Check if filepath starts with prefix | ||
| if prefix and not filepath.startswith(prefix): | ||
| return False | ||
|
|
||
| # Check if filepath ends with suffix pattern | ||
| if suffix: | ||
| # Get the part after the prefix | ||
| if prefix: | ||
| remaining = filepath[len(prefix):].lstrip("/") | ||
| else: | ||
| remaining = filepath | ||
|
|
||
| # The suffix might contain wildcards | ||
| if "*" in suffix: | ||
| # Match the suffix pattern against the end of remaining | ||
| # Try matching against each possible suffix of remaining | ||
| remaining_parts = remaining.split("/") | ||
| suffix_parts = suffix.split("/") | ||
|
|
||
| for i in range(len(remaining_parts) - len(suffix_parts) + 1): | ||
| candidate = "/".join(remaining_parts[i:]) | ||
| if fnmatch.fnmatch(candidate, suffix): | ||
| return True | ||
| return False | ||
| else: | ||
| return remaining.endswith(suffix) or fnmatch.fnmatch(remaining, f"*/{suffix}") | ||
| return True |
There was a problem hiding this comment.
The custom logic for handling ** in glob patterns is quite complex. Python's fnmatch module in Python 3.9+ and later versions natively supports ** for recursive wildcard matching. If the target Python environment is 3.9 or newer, this custom logic might be redundant and could be simplified by relying on fnmatch's built-in capabilities, reducing potential edge cases and improving readability.
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| rm -rf "$FRONTEND_DIR/dist" | ||
| mkdir -p "$(dirname "$DEST_DIR")" | ||
| ditto "$SRC_DIR" "$DEST_DIR" |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
I have read the CLA Document and I hereby sign the CLA |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 Fix all issues with AI agents
In `@apps/backend/core/client.py`:
- Around line 872-891: The parser currently uses fixed thresholds (indent >= 4 /
indent < 4) to detect nested skill properties which fails for 2-space YAML;
change it to track the base indentation of the current list item and use that
for comparisons: when you create current_skill (where you detect a new "-
skill:"/list item) record skill_base_indent = indent, then replace checks that
use "indent >= 4" with "indent > skill_base_indent" when parsing nested keys
like "when" and "paths", and replace the end-of-skills check "indent < 4" with
"indent <= skill_base_indent" to close the skill block; update references to
current_skill and in_skills accordingly so nested properties are recognized
regardless of 2- or 4-space YAML.
- Around line 825-828: Add a single module-level "import re" at the top of
apps/backend/core/client.py and remove all inline "import re" statements inside
the function(s); locate occurrences by searching for uses like "match =
re.search(...)" and the list comprehension "paths = [p.strip().strip("'\"") for
p in match.group(1).split(",")]" and delete the local imports around those
blocks (the inline imports around the match/paths logic and the similar blocks
at the other occurrences) so the code uses the top-level re module instead of
importing inside the function.
- Around line 1555-1561: The prompt text stored in skill_instruction incorrectly
instructs users to manually invoke skills with slash commands; update the string
assigned to skill_instruction to say that the Claude Agent SDK automatically
selects and invokes relevant skills based on SKILL.md and context rather than
using manual slash syntax, e.g., replace the sentence "Use the Skill tool to
invoke skills. Example: `/review`, `/security-audit`" with a brief statement
that skills are automatically selected and invoked by Claude when relevant and
that users should ensure SKILL.md descriptions and context are accurate.
- Around line 925-959: The current matching branch only supports a single "**"
because it splits pattern into parts and only handles len(parts) == 2; update
the logic to handle patterns with multiple "**" occurrences by either: 1)
delegating complex patterns to a glob library with recursive "**" support (e.g.,
wcmatch) before falling back to fnmatch, or 2) converting the pattern (using the
variables pattern and parts) into a proper regex that treats each "**" as a
recursive wildcard and then run that regex against filepath instead of using
fnmatch.fnmatch; ensure the new code still preserves the existing prefix/suffix
checks and returns the same boolean semantics for simple patterns.
In `@apps/frontend/scripts/prebuild-mac.sh`:
- Line 5: The script currently enables "set -e" which stops on errors but
doesn't catch use of undefined variables; update the prebuild script to enable
stricter variable checking by adding "set -u" alongside "set -e" (i.e., change
the bootstrap of the script where "set -e" appears so it becomes "set -euo
pipefail" or at minimum add "-u") to fail fast on unset variables and prevent
subtle build-time bugs.
In `@guides/features/claude-rules-support.md`:
- Around line 145-157: Update the markdown code block that shows the console
output (the block starting with the triple backticks before "Security settings:
.claude_settings.json" and ending with the closing triple backticks) to include
a language specifier by changing the opening fence from ``` to ```text so the
block reads ```text and keep the closing ``` unchanged; this affects the snippet
containing ".claude_settings.json", "Sandbox enabled", and the listed
.claude/rules entries.
- Around line 161-177: The directory-tree code block starting with
".claude/rules/" is missing a language specifier; update the opening code fence
for that block to include "text" (i.e., change the opening ``` to ```text) so
the directory structure is rendered as plain text and preserves formatting.
- Around line 196-215: The fenced code blocks in the three numbered
troubleshooting items ("Check USE_CLAUDE_MD is set", "Check console output for
\"rules matched\"", and "Verify frontmatter format") lack surrounding blank
lines and violate MD031; fix by adding a blank line before and after each fenced
block so each fenced block is separated from the list text (i.e., ensure a blank
line precedes the ```bash / ```text / ```yaml opening fences and a blank line
follows each closing ```), updating those sections in
guides/features/claude-rules-support.md where the headings and list items
appear.
- Around line 130-140: Update the fenced code block that begins with
"Implementation Plan:" so its opening fence includes a language specifier (e.g.,
change the opening ``` to ```text) to enable proper syntax highlighting and
linting; locate the triple-backtick block containing "Implementation Plan:" in
guides/features/claude-rules-support.md and replace the fence accordingly.
- Around line 219-228: The JSON snippet for context.json is missing blank lines
before and after the fenced code block; edit the markdown so the fenced JSON
block that shows "patterns" and "api_route_pattern" is separated by a blank line
above the opening ```json and a blank line after the closing ``` to ensure
proper rendering and readability, and keep the shown keys ("patterns",
"api_route_pattern") intact so readers notice if patterns are empty.
| Check `context.json` in the spec folder - it should contain distilled patterns from your rules: | ||
| ```json | ||
| { | ||
| "patterns": { | ||
| "api_route_pattern": "Use withCsrfProtection wrapper...", | ||
| ... | ||
| } | ||
| } | ||
| ``` | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add blank lines around JSON code block.
🔧 Suggested fix
Check `context.json` in the spec folder - it should contain distilled patterns from your rules:
+
```json
{
"patterns": {
"api_route_pattern": "Use withCsrfProtection wrapper...",
...
}
}If patterns are empty, the rule parsing may have failed silently.
</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
```suggestion
Check `context.json` in the spec folder - it should contain distilled patterns from your rules:
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
220-220: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
🤖 Prompt for AI Agents
In `@guides/features/claude-rules-support.md` around lines 219 - 228, The JSON
snippet for context.json is missing blank lines before and after the fenced code
block; edit the markdown so the fenced JSON block that shows "patterns" and
"api_route_pattern" is separated by a blank line above the opening ```json and a
blank line after the closing ``` to ensure proper rendering and readability, and
keep the shown keys ("patterns", "api_route_pattern") intact so readers notice
if patterns are empty.
Feedback AddressedAll CodeRabbit review feedback has been addressed in the latest commits: Code fixes (
Documentation fixes (
Removed local workaround:
Ready for maintainer review. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@apps/backend/core/client.py`:
- Around line 1034-1064: The current de-duplication uses seen_skills keyed only
by skill name, which collapses distinct requirements across phases/paths; update
the logic in both branches where skills are added (the "if files_to_check is
None" branch and the pattern-match branch inside the loop over all_rules) to
compute a composite key (e.g., combine skill.get("skill",""),
skill.get("phase",""), and a stable representation of skill.get("paths",[])) and
use that composite key instead of just skill_name when checking/adding to
seen_skills so all_required_skills retains separate entries for the same skill
in different phases or with different paths (refer to seen_skills,
all_required_skills, and the for rule_path, patterns, skills, content in
all_rules loop).
- Around line 811-894: The parser treats a nested "paths:" under a structured
skill as a new top-level skill because it only tracks in_paths/in_skills; add a
third state (e.g., in_skill_paths) and use the existing current_skill and
skill_base_indent to detect when "paths:" belongs to the current skill (indent >
skill_base_indent). When you see "paths:" while current_skill is non-None and
indent > skill_base_indent, set in_skill_paths and parse inline arrays into
current_skill["paths"] or collect subsequent "- " list entries into
current_skill["paths"] (stripping quotes) until you hit a line with indent <=
skill_base_indent or a non-list/comment, then clear in_skill_paths; ensure
existing logic still finalizes current_skill into skills when closing the skill
block.
♻️ Duplicate comments (1)
apps/frontend/scripts/prebuild-mac.sh (1)
24-27: Avoid deleting the entiredistdirectory.This still removes unrelated build artifacts; only the target
python-site-packagespath should be cleared.🔧 Safer deletion
- rm -rf "$FRONTEND_DIR/dist" + rm -rf "$DEST_DIR"
apps/backend/core/client.py
Outdated
| matched_rules = [] | ||
| matched_names = [] | ||
| # Track skills by name to avoid duplicates (keep first occurrence) | ||
| seen_skills: set[str] = set() |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Additional Fixes (93446b1)Addressed two more issues from CodeRabbit review:
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@apps/backend/core/client.py`:
- Around line 905-911: The end-of-skills branch currently stops skills parsing
without reprocessing the same line, causing a top-level "paths:" that appears
after require_skills to be ignored; update the branch that sets in_skills =
False / in_skill_paths = False (referencing current_skill, skills, in_skills,
in_skill_paths) to first flush current_skill into skills as now, then inspect
the same stripped line for a top-level "paths:" token and, if present, enter the
paths parsing state (set in_skill_paths = True and in_skills = False) and handle
the line as the start of the paths block (i.e., parse or mark it for the
existing paths-parsing logic) so that rule-level paths are not dropped when
"paths:" follows the skills block.
- Around line 942-975: The current glob-to-regex translation in the loop that
builds regex_pattern (the block handling pattern[i:i+2] == "**") incorrectly
skips a trailing slash after "**", collapsing path separators and overmatching;
change the "**" translation to a segment-aware regex such as "(?:[^/]+/)*" (or
an equivalent that matches zero or more directory segments including their
slashes) and remove the code that advances past a following "/" so slashes are
preserved; keep the rest of the loop logic and final anchoring (regex_pattern =
"^" + regex_pattern + "$") intact.
Quentor1ne
force-pushed
the
feature/claude-rules-support
branch
from
93446b1 to
dfe1248
Compare
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/backend/core/client.py (1)
1-24: Address theruff formatlinting failure.The pipeline reports that
ruff formatcheck failed for this file. Runruff format apps/backend/to auto-fix the formatting issues before merging.
🤖 Fix all issues with AI agents
In `@apps/backend/core/client.py`:
- Around line 744-752: The loop over rule files using rules_dir.rglob("*.md")
currently only appends rules when both paths and rule_content are present (see
_parse_rule_frontmatter and the rules.append line), which silently skips files
with no frontmatter paths (global rules); update the logic to either (a) allow
appending rules that have rule_content even if paths is empty (so global rules
are loaded) or (b) add a logger.debug/logging statement inside the try block
when a file is skipped (i.e., when not (paths and rule_content)) that includes
the rule_path and whether paths or rule_content was missing so users can see why
a rule wasn’t loaded, keeping the exception handling unchanged.
- Around line 673-710: Add unit tests for the _match_glob_pattern function to
cover nested-path glob semantics: create tests that assert
_match_glob_pattern("src/**/*.ts", "src/a/b/c/file.ts") is True,
_match_glob_pattern("**/test/**/*.ts", "foo/test/bar/baz.ts") is True,
_match_glob_pattern("src/**/test/**", "src/deep/nested/test/files") is True, and
_match_glob_pattern("src/**/test/*.ts", "src/contest.ts") is False; put them in
the appropriate test module for apps/backend/core (use the same import path used
by existing tests) and use the test framework’s assertion helpers to make
expectations explicit so future regressions in _match_glob_pattern are caught.
| # Handle ** patterns by converting to regex | ||
| if "**" in pattern: | ||
| # Convert glob pattern to regex | ||
| # Escape regex special characters except * and ? | ||
| regex_pattern = "" | ||
| i = 0 | ||
| while i < len(pattern): | ||
| if pattern[i:i+2] == "**": | ||
| i += 2 | ||
| # Check if ** is followed by / | ||
| if i < len(pattern) and pattern[i] == "/": | ||
| # **/ matches zero or more directory segments (segment-aware) | ||
| # Use (?:.*/)? to match zero or more path segments including slashes | ||
| regex_pattern += "(?:.*/)?" | ||
| i += 1 # Skip the / | ||
| else: | ||
| # ** at end or before non-/ matches remaining path | ||
| regex_pattern += ".*" | ||
| elif pattern[i] == "*": | ||
| # * matches any characters except / | ||
| regex_pattern += "[^/]*" | ||
| i += 1 | ||
| elif pattern[i] == "?": | ||
| # ? matches any single character except / | ||
| regex_pattern += "[^/]" | ||
| i += 1 | ||
| elif pattern[i] in ".^$+{}[]|()": | ||
| # Escape regex special characters | ||
| regex_pattern += "\\" + pattern[i] | ||
| i += 1 | ||
| else: | ||
| regex_pattern += pattern[i] | ||
| i += 1 | ||
|
|
||
| # Match the full path | ||
| regex_pattern = "^" + regex_pattern + "$" | ||
| return bool(re.match(regex_pattern, filepath)) | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Test the glob matching logic with sample patterns
python3 << 'EOF'
import re
def _match_glob_pattern(pattern: str, filepath: str) -> bool:
pattern = pattern.replace("\\", "/").strip("/")
filepath = filepath.replace("\\", "/").strip("/")
if "**" in pattern:
regex_pattern = ""
i = 0
while i < len(pattern):
if pattern[i:i+2] == "**":
i += 2
if i < len(pattern) and pattern[i] == "/":
regex_pattern += "(?:.*/)?"
i += 1
else:
regex_pattern += ".*"
elif pattern[i] == "*":
regex_pattern += "[^/]*"
i += 1
elif pattern[i] == "?":
regex_pattern += "[^/]"
i += 1
elif pattern[i] in ".^$+{}[]|()":
regex_pattern += "\\" + pattern[i]
i += 1
else:
regex_pattern += pattern[i]
i += 1
regex_pattern = "^" + regex_pattern + "$"
return bool(re.match(regex_pattern, filepath))
return False
# Test cases
tests = [
("src/**/*.ts", "src/foo/bar/file.ts", True),
("src/**/*.ts", "src/file.ts", True),
("**/test/**/*.ts", "foo/test/bar/baz.ts", True),
("src/**/test/**", "src/deep/nested/test/files", True),
("src/**/*.ts", "src/contest.ts", True),
("src/**/test/*.ts", "src/contest.ts", False),
]
print("Testing glob patterns:")
all_pass = True
for pattern, filepath, expected in tests:
result = _match_glob_pattern(pattern, filepath)
status = "✓" if result == expected else "✗"
if result != expected:
all_pass = False
# Print the generated regex for debugging
regex_pattern = ""
i = 0
pat_copy = pattern.replace("\\", "/").strip("/")
while i < len(pat_copy):
if pat_copy[i:i+2] == "**":
i += 2
if i < len(pat_copy) and pat_copy[i] == "/":
regex_pattern += "(?:.*/)?"
i += 1
else:
regex_pattern += ".*"
elif pat_copy[i] == "*":
regex_pattern += "[^/]*"
i += 1
elif pat_copy[i] == "?":
regex_pattern += "[^/]"
i += 1
elif pat_copy[i] in ".^$+{}[]|()":
regex_pattern += "\\" + pat_copy[i]
i += 1
else:
regex_pattern += pat_copy[i]
i += 1
regex_pattern = "^" + regex_pattern + "$"
print(f"{status} Pattern '{pattern}' vs '{filepath}': got {result}, expected {expected}")
print(f" Generated regex: {regex_pattern}")
else:
print(f"{status} Pattern '{pattern}' vs '{filepath}': got {result}, expected {expected}")
print(f"\nAll tests passed: {all_pass}")
EOFRepository: AndyMik90/Auto-Claude
Length of output: 151
🏁 Script executed:
sed -n '673,710p' apps/backend/core/client.pyRepository: AndyMik90/Auto-Claude
Length of output: 1531
🏁 Script executed:
#!/bin/bash
# Test simpler cases to identify which pattern causes the timeout
python3 << 'EOF'
import re
import signal
def timeout_handler(signum, frame):
raise TimeoutError("Regex matching timed out")
def _match_glob_pattern(pattern: str, filepath: str) -> bool:
pattern = pattern.replace("\\", "/").strip("/")
filepath = filepath.replace("\\", "/").strip("/")
if "**" in pattern:
regex_pattern = ""
i = 0
while i < len(pattern):
if pattern[i:i+2] == "**":
i += 2
if i < len(pattern) and pattern[i] == "/":
regex_pattern += "(?:.*/)?"
i += 1
else:
regex_pattern += ".*"
elif pattern[i] == "*":
regex_pattern += "[^/]*"
i += 1
elif pattern[i] == "?":
regex_pattern += "[^/]"
i += 1
elif pattern[i] in ".^$+{}[]|()":
regex_pattern += "\\" + pattern[i]
i += 1
else:
regex_pattern += pattern[i]
i += 1
regex_pattern = "^" + regex_pattern + "$"
return bool(re.match(regex_pattern, filepath))
return False
# Test each case individually with timeout
tests = [
("src/**/*.ts", "src/foo/bar/file.ts"),
("src/**/*.ts", "src/file.ts"),
("**/test/**/*.ts", "foo/test/bar/baz.ts"),
]
print("Testing individual cases:")
for pattern, filepath in tests:
try:
signal.signal(signal.SIGALRM, timeout_handler)
signal.alarm(5) # 5 second timeout per test
result = _match_glob_pattern(pattern, filepath)
signal.alarm(0) # Cancel alarm
print(f"✓ '{pattern}' vs '{filepath}': {result}")
except TimeoutError:
print(f"✗ '{pattern}' vs '{filepath}': TIMEOUT (catastrophic backtracking)")
except Exception as e:
print(f"✗ '{pattern}' vs '{filepath}': ERROR - {e}")
EOFRepository: AndyMik90/Auto-Claude
Length of output: 227
🏁 Script executed:
find apps/backend -type f -name "*.py" | xargs grep -l "_match_glob_pattern\|test.*glob" | head -20Repository: AndyMik90/Auto-Claude
Length of output: 132
🏁 Script executed:
sed -n '650,715p' apps/backend/core/client.pyRepository: AndyMik90/Auto-Claude
Length of output: 2520
🏁 Script executed:
grep -A 20 -B 5 "_match_glob_pattern\|test.*glob" apps/backend/analysis/test_discovery.py | head -80Repository: AndyMik90/Auto-Claude
Length of output: 1022
🏁 Script executed:
sed -n '650,730p' apps/backend/core/client.pyRepository: AndyMik90/Auto-Claude
Length of output: 2966
🏁 Script executed:
grep -r "_match_glob_pattern" apps/backend --include="*.py" -B 2 -A 2 | head -40Repository: AndyMik90/Auto-Claude
Length of output: 714
Add unit tests for the _match_glob_pattern function to ensure glob matching handles nested paths correctly.
The function lacks test coverage. While manual testing shows basic patterns work (e.g., src/**/*.ts correctly matches src/foo/bar/file.ts), add tests to document expected behavior and catch edge cases:
src/**/*.tsmatchingsrc/a/b/c/file.ts**/test/**/*.tsmatchingfoo/test/bar/baz.tssrc/**/test/**matchingsrc/deep/nested/test/filessrc/**/test/*.tsNOT matchingsrc/contest.ts
🤖 Prompt for AI Agents
In `@apps/backend/core/client.py` around lines 673 - 710, Add unit tests for the
_match_glob_pattern function to cover nested-path glob semantics: create tests
that assert _match_glob_pattern("src/**/*.ts", "src/a/b/c/file.ts") is True,
_match_glob_pattern("**/test/**/*.ts", "foo/test/bar/baz.ts") is True,
_match_glob_pattern("src/**/test/**", "src/deep/nested/test/files") is True, and
_match_glob_pattern("src/**/test/*.ts", "src/contest.ts") is False; put them in
the appropriate test module for apps/backend/core (use the same import path used
by existing tests) and use the test framework’s assertion helpers to make
expectations explicit so future regressions in _match_glob_pattern are caught.
|
Long story short. I love the idea of Auto Claude. And of course, I started using it. have to admit that skills are so fun and uncharted territory, that it's hard to say how they should actually fit into the picture in the long run. Maybe they need some UI at some point. Cheers! |
Implements Claude Code's convention of path-based rules in .claude/rules/. Rules are markdown files with YAML frontmatter containing glob patterns that determine which files trigger the rule. Features: - Parses YAML frontmatter with 'paths' arrays from rule files - Matches glob patterns including ** for directory depth - Loads rules based on files in implementation_plan.json - Falls back to loading all rules during planning phases - New USE_CLAUDE_RULES env var (defaults to following USE_CLAUDE_MD) - UI setting support via useClaudeRules in ProjectSettings Example rule format: --- paths: - src/app/api/**/*.ts - src/components/**/*.tsx --- # Rule content here... This enables Auto-Claude to respect project-specific coding standards, security patterns, and conventions defined in .claude/rules/. Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Parse structured require_skills format in rule frontmatter - Support `when` timing: planning, per_subtask, end_of_coding, qa_phase - Support optional `paths` filter for narrowing skill execution - Generate agent-aware skill instructions based on current phase - Add prebuild-mac.sh to fix EPERM copyfile issues on macOS Docs: guides/features/claude-rules-support.md Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Move import re/fnmatch to top of file (no inline imports) - Fix indent parsing to use dynamic base indent (supports 2-space YAML) - Improve glob matching with regex to support multiple ** patterns - Update skill instruction text to reflect automatic invocation - Add set -euo pipefail to prebuild-mac.sh for stricter checking - Add language specifiers to markdown code blocks - Add blank lines around fenced code blocks (MD031) Co-Authored-By: Claude Opus 4.5 <[email protected]>
This removes the prebuild-mac.sh script and package.json changes that were a local workaround for EPERM copyfile issues. These are not part of the upstream feature contribution. Co-Authored-By: Claude Opus 4.5 <[email protected]>
Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Add in_skill_paths state to properly parse multi-line paths: lists under structured skills (fixes silent parsing failure) - Change skill deduplication to use composite key (skill, when, paths) instead of just skill name (preserves different configurations of same skill across phases) Co-Authored-By: Claude Opus 4.5 <[email protected]>
- When skills block ends, reprocess line for top-level paths: that may follow require_skills: in frontmatter - Use segment-aware regex (?:.*/)? for **/ patterns instead of greedy .* to prevent overmatching across path segments Co-Authored-By: Claude Opus 4.5 <[email protected]>
Co-Authored-By: Claude Opus 4.5 <[email protected]>
When rules in .claude/rules/ have content but no paths: frontmatter, they are now logged at debug level for visibility. Co-Authored-By: Claude Opus 4.5 <[email protected]>
Tests cover nested-path glob semantics: - src/**/*.ts matching nested directories - **/test/**/*.ts with leading/trailing double star - src/**/test/** with deeply nested paths - src/**/test/*.ts NOT matching partial names (contest vs test) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Quentor1ne
force-pushed
the
feature/claude-rules-support
branch
from
84e3e74 to
b955c85
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/backend/core/client.py`:
- Around line 570-576: When parsing frontmatter in apps/backend/core/client.py,
the in_paths block currently ends on encountering a non-list top-level line but
doesn't reprocess that same 'stripped' line, causing 'require_skills' (and other
keys) to be skipped; modify the logic around the in_paths branch so that when
you detect a non-list top-level line you set in_paths = False and immediately
re-evaluate the same 'stripped' value against the top-level key handlers (i.e.,
don't consume the line). Concretely, change the current "elif in_paths: ... elif
stripped and not stripped.startswith('#'): in_paths = False" flow so the
non-list branch either uses a continue to re-run the top-level checks on the
same line or replaces the elif with an if and falls through to the following
in_skills / require_skills checks; refer to the variables/functions 'in_paths',
'in_skills', 'paths', and the 'stripped' line handling to locate the spot to
adjust.
When parsing rule frontmatter, the require_skills key was being skipped if it immediately followed the paths array because: 1. The paths block didn't continue after processing 2. The require_skills check used elif instead of if This fix: - Adds continue statements after processing paths: and require_skills: keys - Changes elif to if for require_skills:, in_paths, and in_skills checks - Adds continue after processing path list items - Allows proper fall-through when paths array ends to check for other keys Co-Authored-By: Claude Opus 4.5 <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/backend/core/client.py`:
- Around line 689-728: Add unit tests in the existing test_client.py that
exercise the glob-matching branch in client.py which converts "**" patterns to
regex (the code that builds regex_pattern and returns re.match or falls back to
fnmatch.fnmatch); include positive and negative assertions for these edge cases:
pattern "**" (should match any non-empty and empty filepath as applicable),
pattern "**.ts" (no slash after ** — ensure it matches filenames ending with .ts
but not across path separators), an explicit empty filepath input, and negative
cases for each to guard regressions. Use the same matching entry-point used by
current tests (call the function that takes pattern and filepath) and assert
True/False accordingly.
Addresses CodeRabbit review feedback: - Test standalone ** pattern matches any path (including empty) - Test **.ts pattern without slash matches .ts files - Test empty filepath with specific patterns - Test empty pattern edge case Co-Authored-By: Claude Opus 4.5 <[email protected]>
AndyMik90
force-pushed
the
develop
branch
2 times, most recently
from
67a743f to
e83e445
Compare
3 participants
Base Branch
developbranch (required for all feature/fix PRs)main(hotfix only - maintainers)Description
Adds
require_skillsfrontmatter support to.claude/rules/with timing control (whenfield). Rules can now specify when skills should run: during planning, per subtask, end of coding, or QA phase. Also includes a macOS build fix for EPERM copyfile errors.Related Issue
N/A
Type of Change
Area
Commit Message Format
feat(rules): add require_skills with timing control to rulesChecklist
developbranchencoding="utf-8"for text filesPlatform Testing Checklist
platform/module instead of directprocess.platformchecksfindExecutable()or platform abstractions)CI/Testing Requirements
Screenshots
N/A - Backend feature, no UI changes.
Feature Toggle
Breaking Changes
Breaking: No
Summary by CodeRabbit
New Features
Documentation
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.