fix: skill-guardrail#8
Conversation
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- README.md: 21-row class mapping table, before/after, Hebrew example - skills/rtl-validator/SKILL.md: 28-entry violation table, rtl-ok suppression - skills/rtl-fix/SKILL.md: full replacement mapping, cannot-auto-fix list - cheatsheet/tailwind-rtl.md: 9 sections, Flutter equivalents, Hebrew checkout form - CI/rtl-check.yml: PR blocker on rtl-unsafe patterns, auto PR comment - LICENSE: MIT 2026 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 59 minutes and 35 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![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: ac0e82b549
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| VIOLATIONS=$(rg --glob "*.tsx" --glob "*.jsx" --glob "*.css" --glob "*.vue" \ | ||
| --line-number \ | ||
| --no-heading \ | ||
| --color never \ | ||
| "$PATTERN" \ | ||
| src/ components/ pages/ app/ styles/ 2>/dev/null \ |
There was a problem hiding this comment.
The RTL scan command only searches src/ components/ pages/ app/ styles/ and suppresses missing-path errors, so projects that keep UI code elsewhere (for example packages/*, apps/*, or lib/*) will report zero violations and incorrectly pass CI. This contradicts the step comment that it scans PR changes with a fallback, and it allows physical-direction regressions to merge unnoticed.
Useful? React with 👍 / 👎.
| # float-left float-right — physical float | ||
| # left-{n} right-{n} — physical inset positioning (when directional) | ||
|
|
||
| PATTERN='\bml-[0-9a-z\[]|\bmr-[0-9a-z\[]|\bpl-[0-9a-z\[]|\bpr-[0-9a-z\[]|text-left\b|text-right\b|border-l-|border-r-|\brounded-tl\b|\brounded-tr\b|\brounded-bl\b|\brounded-br\b|float-left\b|float-right\b' |
There was a problem hiding this comment.
The regex does not include left-* or right-*, even though the check description says those classes are enforced. As a result, physical positioning utilities can be introduced without being flagged, creating false negatives for one of the directional patterns this workflow is intended to block.
Useful? React with 👍 / 👎.
| @@ -0,0 +1,174 @@ | |||
| name: RTL Check | |||
There was a problem hiding this comment.
ci
Adding CI/rtl-check.yml introduces a case-only directory distinction from the existing ci/ tree, which breaks or behaves inconsistently on case-insensitive filesystems (default macOS/Windows). Contributors on those environments can hit checkout/path conflicts, so this should be moved to a non-colliding path (for example .github/workflows/ or ci/).
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR expands the RTL-first dev kit by adding two AI “skills” (validation + auto-fix), a comprehensive Tailwind RTL cheatsheet, and CI/security automation scaffolding to detect physical-direction regressions.
Changes:
- Add
rtl-validatorandrtl-fixskills with mappings, suppression rules, and examples. - Add a Tailwind 4.x RTL logical-properties cheatsheet and refresh README/SKILL metadata.
- Introduce CI workflow template for RTL scanning plus a new repo security scan workflow.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| skills/rtl-validator/SKILL.md | New skill spec for detecting RTL-violating physical classes/properties. |
| skills/rtl-fix/SKILL.md | New skill spec for mechanically converting physical → logical classes/properties. |
| cheatsheet/tailwind-rtl.md | New Tailwind 4.x RTL reference doc with mappings and examples. |
| SKILL.md | Updates skill metadata/triggers and adds a security guardrail note. |
| README.md | Rewrites README with installation guidance and references to new artifacts. |
| LICENSE | Updates copyright holder name. |
| CI/rtl-check.yml | Adds a workflow template intended to block physical-direction classes in PRs. |
| .github/workflows/security-scan.yml | Adds Semgrep + Trivy security scanning on a self-hosted runner. |
| .github/FUNDING.yml | Adds funding configuration. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Classes checked: | ||
| # ml-{n} mr-{n} pl-{n} pr-{n} — physical margin/padding | ||
| # text-left text-right — physical text alignment | ||
| # border-l- border-r- — physical border sides | ||
| # rounded-tl rounded-tr — physical border radius (corners) | ||
| # rounded-bl rounded-br | ||
| # float-left float-right — physical float | ||
| # left-{n} right-{n} — physical inset positioning (when directional) | ||
|
|
||
| PATTERN='\bml-[0-9a-z\[]|\bmr-[0-9a-z\[]|\bpl-[0-9a-z\[]|\bpr-[0-9a-z\[]|text-left\b|text-right\b|border-l-|border-r-|\brounded-tl\b|\brounded-tr\b|\brounded-bl\b|\brounded-br\b|float-left\b|float-right\b' |
There was a problem hiding this comment.
The comment says left-{n} / right-{n} are checked, but the PATTERN does not include them. Either add \bleft- / \bright- (and handle the documented exception for left-0 right-0), or remove the claim from the comments so behavior matches documentation.
| # Run ripgrep: | ||
| # - search only changed files in the PR (fallback: all source files) | ||
| # - exclude lines containing "rtl-ok" suppression comment | ||
| # - show file, line number, and matching content | ||
| VIOLATIONS=$(rg --glob "*.tsx" --glob "*.jsx" --glob "*.css" --glob "*.vue" \ | ||
| --line-number \ | ||
| --no-heading \ | ||
| --color never \ | ||
| "$PATTERN" \ | ||
| src/ components/ pages/ app/ styles/ 2>/dev/null \ | ||
| | grep -v "rtl-ok" \ |
There was a problem hiding this comment.
The scan is limited to src/ components/ pages/ app/ styles/, which can miss violations in other directories (and doesn’t implement the stated “search only changed files in the PR”). Consider scanning the whole repo (excluding vendor/build dirs) or deriving the changed file list from the PR and scanning only those paths.
| name: RTL Logical Properties Check | ||
| runs-on: ubuntu-latest | ||
| permissions: | ||
| contents: read |
There was a problem hiding this comment.
This workflow uses github.rest.issues.createComment, which typically requires issues: write permission. Currently only pull-requests: write is granted, so posting the PR comment may fail with a permissions error. Add issues: write (or switch to a PR-review comment API that matches the granted scope).
| contents: read | |
| contents: read | |
| issues: write |
| | grep -v "rtl-ok" \ | ||
| | grep -v "//.*rtl-ok" \ | ||
| || true) |
There was a problem hiding this comment.
grep -v "rtl-ok" will exclude any line containing that substring (including string literals), not just suppression comments. If suppression is meant to be // rtl-ok, prefer a tighter filter (e.g., match comment patterns) and drop the redundant second grep once the filter is precise.
| - uses: actions/checkout@v4 | ||
| - name: Semgrep SAST | ||
| run: | | ||
| pip3 install --user semgrep 2>/dev/null || true |
There was a problem hiding this comment.
pip3 install --user semgrep does not guarantee semgrep is on PATH (user installs often land in ~/.local/bin). On a fresh runner this can make semgrep scan fail even though the install succeeded. Prefer installing in a way that ensures availability (e.g., add the user bin dir to PATH, use pipx, or invoke via python -m semgrep if supported).
| pip3 install --user semgrep 2>/dev/null || true | |
| pip3 install --user semgrep 2>/dev/null || true | |
| export PATH="$HOME/.local/bin:$PATH" |
| trivy: | ||
| runs-on: [self-hosted, linux, x64, pop-os] | ||
| steps: | ||
| - uses: actions/checkout@v4 |
There was a problem hiding this comment.
The Trivy job assumes trivy is already installed on the self-hosted runner. If the runner is rebuilt or used elsewhere, this job will fail with command not found. Consider installing Trivy in the workflow (or explicitly documenting the runner prerequisites and pinning the expected Trivy version).
| - uses: actions/checkout@v4 | |
| - uses: actions/checkout@v4 | |
| - name: Install Trivy | |
| run: | | |
| TRIVY_VERSION=0.51.2 | |
| curl -sfL https://raw.githubusercontent.com/aquasecurity/trivy/main/contrib/install.sh | sudo sh -s -- -b /usr/local/bin "v${TRIVY_VERSION}" |
| | `skills/rtl-validator/SKILL.md` | AI skill: validates RTL compliance during code review | | ||
| | `skills/rtl-fix/SKILL.md` | AI skill: auto-converts physical to logical properties | | ||
| | `cheatsheet/tailwind-rtl.md` | Complete Tailwind 4.x logical property reference | | ||
| | `CI/rtl-check.yml` | GitHub Actions: blocks PRs containing physical direction classes | |
There was a problem hiding this comment.
README claims CI/rtl-check.yml “blocks PRs containing physical direction classes”, but the current workflow regex/directory scoping won’t catch many real violations. Either tighten the workflow so the statement is accurate, or soften the wording to reflect current coverage/limitations.
| | `CI/rtl-check.yml` | GitHub Actions: blocks PRs containing physical direction classes | | |
| | `CI/rtl-check.yml` | GitHub Actions: PR check for flagged physical direction classes in covered paths | |
| # ml-{n} mr-{n} pl-{n} pr-{n} — physical margin/padding | ||
| # text-left text-right — physical text alignment | ||
| # border-l- border-r- — physical border sides | ||
| # rounded-tl rounded-tr — physical border radius (corners) | ||
| # rounded-bl rounded-br | ||
| # float-left float-right — physical float | ||
| # left-{n} right-{n} — physical inset positioning (when directional) | ||
|
|
||
| PATTERN='\bml-[0-9a-z\[]|\bmr-[0-9a-z\[]|\bpl-[0-9a-z\[]|\bpr-[0-9a-z\[]|text-left\b|text-right\b|border-l-|border-r-|\brounded-tl\b|\brounded-tr\b|\brounded-bl\b|\brounded-br\b|float-left\b|float-right\b' |
There was a problem hiding this comment.
The scan regex misses common variants, so the workflow will silently pass on violations (e.g., rounded-tl-md, rounded-tr-lg, border-l without a trailing dash, negative classes like -ml-4, and ml-1/2 / other non-alnum modifiers). Update the pattern to cover these forms (or switch to a safer tokenization approach) so the check actually enforces the documented policy.
| # ml-{n} mr-{n} pl-{n} pr-{n} — physical margin/padding | |
| # text-left text-right — physical text alignment | |
| # border-l- border-r- — physical border sides | |
| # rounded-tl rounded-tr — physical border radius (corners) | |
| # rounded-bl rounded-br | |
| # float-left float-right — physical float | |
| # left-{n} right-{n} — physical inset positioning (when directional) | |
| PATTERN='\bml-[0-9a-z\[]|\bmr-[0-9a-z\[]|\bpl-[0-9a-z\[]|\bpr-[0-9a-z\[]|text-left\b|text-right\b|border-l-|border-r-|\brounded-tl\b|\brounded-tr\b|\brounded-bl\b|\brounded-br\b|float-left\b|float-right\b' | |
| # ml-* mr-* pl-* pr-* — physical margin/padding (including negatives, fractions, arbitrary values) | |
| # text-left text-right — physical text alignment | |
| # border-l border-r — physical border sides (bare or with modifiers) | |
| # rounded-tl rounded-tr — physical border radius (corners, bare or with modifiers) | |
| # rounded-bl rounded-br | |
| # float-left float-right — physical float | |
| # left-* right-* — physical inset positioning (when directional) | |
| PATTERN="(^|[^[:alnum:]_/-])(-?(ml|mr|pl|pr|left|right)-[^[:space:]\"'\`<>{}]+|text-(left|right)([^[:alnum:]_-]|$)|border-(l|r)(-[^[:space:]\"'\`<>{}]+)?([^[:alnum:]_-]|$)|rounded-(tl|tr|bl|br)(-[^[:space:]\"'\`<>{}]+)?([^[:alnum:]_-]|$)|float-(left|right)([^[:alnum:]_-]|$))" |
Automated: fix/skill-guardrail