chore: merge-remote-bootstrap#6
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>
|
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 38 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 (8)
✨ 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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 64648d421d
ℹ️ 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".
| # 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.
Add left/right utilities to violation regex
The scan pattern never matches physical inset classes like left-0 or right-4, even though the workflow comments and skill docs state these should be blocked. As a result, PRs can introduce directional positioning regressions that this check reports as clean, which defeats a core RTL-safety guarantee.
Useful? React with 👍 / 👎.
| --no-heading \ | ||
| --color never \ | ||
| "$PATTERN" \ | ||
| src/ components/ pages/ app/ styles/ 2>/dev/null \ |
There was a problem hiding this comment.
Scan repository files instead of fixed framework folders
The rg command is restricted to src/ components/ pages/ app/ styles/, so violations in common layouts outside these folders (for example monorepo packages, lib/, or root-level feature dirs) are silently skipped. Since the workflow triggers on all *.tsx/*.jsx/*.css/*.vue changes, this causes frequent false negatives and allows RTL-breaking classes to merge unnoticed.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR bootstraps an RTL-first “dev kit” by adding AI skill prompts, a Tailwind RTL reference cheatsheet, and CI/security workflows intended to detect physical-direction regressions.
Changes:
- Add
rtl-validatorandrtl-fixskill definitions for AI-assisted review and conversion to logical properties. - Add a Tailwind 4.x RTL logical-properties cheatsheet and expand the repo README with installation/usage guidance.
- Add an RTL-check workflow template and a security scanning workflow (Semgrep/Trivy), plus small repo metadata updates.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| skills/rtl-validator/SKILL.md | New AI “validator” skill prompt and rules/mapping for detecting physical-direction usage. |
| skills/rtl-fix/SKILL.md | New AI “fixer” skill prompt and conversion mapping, including edge-case guidance. |
| cheatsheet/tailwind-rtl.md | New long-form Tailwind 4.x RTL reference for logical utilities and patterns. |
| README.md | Rewritten project README with positioning, included components, and install instructions. |
| LICENSE | Updates copyright holder name. |
| CI/rtl-check.yml | Adds an RTL check workflow template that greps for physical-direction patterns. |
| .github/workflows/security-scan.yml | Adds Semgrep + Trivy security scan workflow on the self-hosted runner. |
| .github/FUNDING.yml | Adds funding configuration. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| steps: | ||
| - uses: actions/checkout@v4 | ||
| - name: Semgrep SAST |
There was a problem hiding this comment.
actions/checkout@v4 is not pinned to a commit SHA here. This repo pins the checkout action in .github/workflows/ci.yml; consider pinning it in this workflow too for consistency and better supply-chain security.
| run: | | ||
| pip3 install --user semgrep 2>/dev/null || true | ||
| semgrep scan . --config=auto --error --severity ERROR | ||
|
|
||
| trivy: | ||
| runs-on: [self-hosted, linux, x64, pop-os] | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| - name: Trivy vulnerability scan | ||
| run: trivy fs . --severity HIGH,CRITICAL --exit-code 1 |
There was a problem hiding this comment.
pip3 install --user semgrep 2>/dev/null || true hides installation failures, which makes the subsequent semgrep scan failure harder to diagnose. Also, trivy fs ... assumes Trivy is already installed on the self-hosted runner; if not, this job will fail immediately. Consider explicitly installing/pinning these tools (or using the official setup/actions) and avoid suppressing install errors.
| run: | | |
| pip3 install --user semgrep 2>/dev/null || true | |
| semgrep scan . --config=auto --error --severity ERROR | |
| trivy: | |
| runs-on: [self-hosted, linux, x64, pop-os] | |
| steps: | |
| - uses: actions/checkout@v4 | |
| - name: Trivy vulnerability scan | |
| run: trivy fs . --severity HIGH,CRITICAL --exit-code 1 | |
| uses: semgrep/semgrep-action@v1 | |
| with: | |
| config: auto | |
| generateSarif: "false" | |
| auditOn: push | |
| trivy: | |
| runs-on: [self-hosted, linux, x64, pop-os] | |
| steps: | |
| - uses: actions/checkout@v4 | |
| - name: Trivy vulnerability scan | |
| uses: aquasecurity/trivy-action@0.24.0 | |
| with: | |
| scan-type: fs | |
| scan-ref: . | |
| severity: HIGH,CRITICAL | |
| exit-code: '1' |
| ## False Positive Exceptions — Do NOT Flag | ||
|
|
||
| 1. `left-0 right-0` (both physical edges set) — this is centering / full-width stretch, not directional | ||
| 2. `margin-left: auto` combined with `margin-right: auto` — this is horizontal centering | ||
| 3. `translate-x-{n}` in animation without context — warn, do not block | ||
| 4. `float: left` inside a container that has `direction: ltr` explicitly set | ||
| 5. Storybook decorators that intentionally test LTR-only variants |
There was a problem hiding this comment.
The "False Positive Exceptions — Do NOT Flag" list conflicts with the Prime Directive above ("Any physical direction class is an RTL violation" unless dir="ltr" + // rtl-ok). Either move these items into the Prime Directive as explicit allowed patterns, or require dir="ltr" + // rtl-ok for them as well so the guidance is consistent.
| ## False Positive Exceptions — Do NOT Flag | |
| 1. `left-0 right-0` (both physical edges set) — this is centering / full-width stretch, not directional | |
| 2. `margin-left: auto` combined with `margin-right: auto` — this is horizontal centering | |
| 3. `translate-x-{n}` in animation without context — warn, do not block | |
| 4. `float: left` inside a container that has `direction: ltr` explicitly set | |
| 5. Storybook decorators that intentionally test LTR-only variants | |
| ## Allowed Exceptions / Suppressions | |
| Physical-direction patterns are still violations by default. Only suppress them when the code is intentionally LTR-only and includes both explicit LTR context (for example `dir="ltr"` or `direction: ltr`) and a `// rtl-ok` comment. | |
| 1. `left-0 right-0` (both physical edges set) — allowed only with explicit LTR context + `// rtl-ok` | |
| 2. `margin-left: auto` combined with `margin-right: auto` — allowed only with explicit LTR context + `// rtl-ok` | |
| 3. `translate-x-{n}` in animation without context — warn, do not block | |
| 4. `float: left` inside a container that has `direction: ltr` explicitly set — allowed only with `// rtl-ok` | |
| 5. Storybook decorators that intentionally test LTR-only variants — allowed only with explicit LTR context + `// rtl-ok` |
| When both `left-0` and `right-0` appear together, this is a full-width stretch, | ||
| not a directional anchor. Replace both with `inset-x-0` (which is already logical | ||
| for block axis stretching). | ||
|
|
||
| ```jsx | ||
| // BEFORE — full-width overlay, not directional | ||
| <div className="absolute left-0 right-0 top-0 bottom-0 bg-black/50" /> | ||
|
|
||
| // AFTER — inset-0 is already logical (covers all four sides) |
There was a problem hiding this comment.
This section says to replace left-0 right-0 with inset-x-0, but the example replaces with inset-0. Also, inset-x-0 alone would not cover the top-0 bottom-0 case shown. Please align the rule text and examples (e.g., clarify when to use inset-x-0 vs inset-0).
| When both `left-0` and `right-0` appear together, this is a full-width stretch, | |
| not a directional anchor. Replace both with `inset-x-0` (which is already logical | |
| for block axis stretching). | |
| ```jsx | |
| // BEFORE — full-width overlay, not directional | |
| <div className="absolute left-0 right-0 top-0 bottom-0 bg-black/50" /> | |
| // AFTER — inset-0 is already logical (covers all four sides) | |
| When `left-0` and `right-0` appear together, this is an inline-axis stretch, | |
| not a directional anchor. Replace that pair with `inset-x-0`. If the element is | |
| also stretched vertically with `top-0 bottom-0`, replace all four utilities with | |
| `inset-0`. | |
| ```jsx | |
| // BEFORE — full overlay, stretched on all four sides | |
| <div className="absolute left-0 right-0 top-0 bottom-0 bg-black/50" /> | |
| // AFTER — inset-0 is already logical and covers all four sides |
| @@ -0,0 +1,2 @@ | |||
| github: Nadav011 | |||
There was a problem hiding this comment.
The funding handle (github: Nadav011) doesn't match the author identity used elsewhere in this PR (LICENSE uses "Nadav Cohen" and README links to github.com/nadavcohen). If the GitHub Sponsors handle is different intentionally, consider adding a brief note; otherwise update it to the correct account to avoid misdirected sponsorships.
| github: Nadav011 | |
| github: nadavcohen |
| # 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 doesn't match the "Classes checked" comment list: left-*/right-* are mentioned but not included; rounded-tl/rounded-tr patterns only match the exact class (missing common rounded-tl-lg etc.); and border-l / border-r without a trailing hyphen won't be detected. This will let many physical-direction classes through CI; expand the pattern to cover the documented cases.
| 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' | |
| PATTERN='\bml-[0-9a-z\[]|\bmr-[0-9a-z\[]|\bpl-[0-9a-z\[]|\bpr-[0-9a-z\[]|text-left\b|text-right\b|\bborder-l(?:\b|-)|\bborder-r(?:\b|-)|\brounded-tl(?:\b|-)|\brounded-tr(?:\b|-)|\brounded-bl(?:\b|-)|\brounded-br(?:\b|-)|float-left\b|float-right\b|\bleft-[0-9a-z\[]|\bright-[0-9a-z\[]' |
| # 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" \ | ||
| | grep -v "//.*rtl-ok" \ | ||
| || true) |
There was a problem hiding this comment.
The comment says this step will "search only changed files in the PR", but the rg invocation scans hard-coded directories (src/ components/ pages/ app/ styles/). This can miss violations in other paths touched by the PR and makes the comment misleading. Consider either actually deriving the file list from the PR diff, or update the comment and directory list to match intended coverage.
| # Determine the replacement hint | ||
| HINT="" | ||
| if echo "$CONTENT" | grep -qE '\bml-'; then HINT="→ use ms-* (margin-inline-start)"; fi | ||
| if echo "$CONTENT" | grep -qE '\bmr-'; then HINT="→ use me-* (margin-inline-end)"; fi | ||
| if echo "$CONTENT" | grep -qE '\bpl-'; then HINT="→ use ps-* (padding-inline-start)"; fi | ||
| if echo "$CONTENT" | grep -qE '\bpr-'; then HINT="→ use pe-* (padding-inline-end)"; fi | ||
| if echo "$CONTENT" | grep -qE 'text-left'; then HINT="→ use text-start"; fi | ||
| if echo "$CONTENT" | grep -qE 'text-right'; then HINT="→ use text-end"; fi | ||
| if echo "$CONTENT" | grep -qE 'border-l-'; then HINT="→ use border-s-* (border-inline-start)"; fi | ||
| if echo "$CONTENT" | grep -qE 'border-r-'; then HINT="→ use border-e-* (border-inline-end)"; fi | ||
| if echo "$CONTENT" | grep -qE 'rounded-tl'; then HINT="→ use rounded-ss-* (border-start-start-radius)"; fi | ||
| if echo "$CONTENT" | grep -qE 'rounded-tr'; then HINT="→ use rounded-se-* (border-start-end-radius)"; fi | ||
| if echo "$CONTENT" | grep -qE 'rounded-bl'; then HINT="→ use rounded-es-* (border-end-start-radius)"; fi | ||
| if echo "$CONTENT" | grep -qE 'rounded-br'; then HINT="→ use rounded-ee-* (border-end-end-radius)"; fi | ||
| if echo "$CONTENT" | grep -qE 'float-left'; then HINT="→ use float-start"; fi | ||
| if echo "$CONTENT" | grep -qE 'float-right'; then HINT="→ use float-end"; fi | ||
|
|
||
| echo "${FILE_LINE} ${HINT}" >> $GITHUB_STEP_SUMMARY |
There was a problem hiding this comment.
The generated step summary drops the matched text (CONTENT) and only prints file:line plus a single hint, which makes it hard to see what was actually flagged. Also, HINT is overwritten if a line contains multiple violations, so the output can be incomplete. Include the matched snippet/class in the summary and/or accumulate multiple hints per line.
| # Determine the replacement hint | |
| HINT="" | |
| if echo "$CONTENT" | grep -qE '\bml-'; then HINT="→ use ms-* (margin-inline-start)"; fi | |
| if echo "$CONTENT" | grep -qE '\bmr-'; then HINT="→ use me-* (margin-inline-end)"; fi | |
| if echo "$CONTENT" | grep -qE '\bpl-'; then HINT="→ use ps-* (padding-inline-start)"; fi | |
| if echo "$CONTENT" | grep -qE '\bpr-'; then HINT="→ use pe-* (padding-inline-end)"; fi | |
| if echo "$CONTENT" | grep -qE 'text-left'; then HINT="→ use text-start"; fi | |
| if echo "$CONTENT" | grep -qE 'text-right'; then HINT="→ use text-end"; fi | |
| if echo "$CONTENT" | grep -qE 'border-l-'; then HINT="→ use border-s-* (border-inline-start)"; fi | |
| if echo "$CONTENT" | grep -qE 'border-r-'; then HINT="→ use border-e-* (border-inline-end)"; fi | |
| if echo "$CONTENT" | grep -qE 'rounded-tl'; then HINT="→ use rounded-ss-* (border-start-start-radius)"; fi | |
| if echo "$CONTENT" | grep -qE 'rounded-tr'; then HINT="→ use rounded-se-* (border-start-end-radius)"; fi | |
| if echo "$CONTENT" | grep -qE 'rounded-bl'; then HINT="→ use rounded-es-* (border-end-start-radius)"; fi | |
| if echo "$CONTENT" | grep -qE 'rounded-br'; then HINT="→ use rounded-ee-* (border-end-end-radius)"; fi | |
| if echo "$CONTENT" | grep -qE 'float-left'; then HINT="→ use float-start"; fi | |
| if echo "$CONTENT" | grep -qE 'float-right'; then HINT="→ use float-end"; fi | |
| echo "${FILE_LINE} ${HINT}" >> $GITHUB_STEP_SUMMARY | |
| # Determine the replacement hint(s) | |
| HINTS="" | |
| if echo "$CONTENT" | grep -qE '\bml-'; then HINTS="${HINTS}→ use ms-* (margin-inline-start)\n"; fi | |
| if echo "$CONTENT" | grep -qE '\bmr-'; then HINTS="${HINTS}→ use me-* (margin-inline-end)\n"; fi | |
| if echo "$CONTENT" | grep -qE '\bpl-'; then HINTS="${HINTS}→ use ps-* (padding-inline-start)\n"; fi | |
| if echo "$CONTENT" | grep -qE '\bpr-'; then HINTS="${HINTS}→ use pe-* (padding-inline-end)\n"; fi | |
| if echo "$CONTENT" | grep -qE 'text-left'; then HINTS="${HINTS}→ use text-start\n"; fi | |
| if echo "$CONTENT" | grep -qE 'text-right'; then HINTS="${HINTS}→ use text-end\n"; fi | |
| if echo "$CONTENT" | grep -qE 'border-l-'; then HINTS="${HINTS}→ use border-s-* (border-inline-start)\n"; fi | |
| if echo "$CONTENT" | grep -qE 'border-r-'; then HINTS="${HINTS}→ use border-e-* (border-inline-end)\n"; fi | |
| if echo "$CONTENT" | grep -qE 'rounded-tl'; then HINTS="${HINTS}→ use rounded-ss-* (border-start-start-radius)\n"; fi | |
| if echo "$CONTENT" | grep -qE 'rounded-tr'; then HINTS="${HINTS}→ use rounded-se-* (border-start-end-radius)\n"; fi | |
| if echo "$CONTENT" | grep -qE 'rounded-bl'; then HINTS="${HINTS}→ use rounded-es-* (border-end-start-radius)\n"; fi | |
| if echo "$CONTENT" | grep -qE 'rounded-br'; then HINTS="${HINTS}→ use rounded-ee-* (border-end-end-radius)\n"; fi | |
| if echo "$CONTENT" | grep -qE 'float-left'; then HINTS="${HINTS}→ use float-start\n"; fi | |
| if echo "$CONTENT" | grep -qE 'float-right'; then HINTS="${HINTS}→ use float-end\n"; fi | |
| if [ -n "$HINTS" ]; then | |
| printf '%s\n matched: %s\n%s\n' "$FILE_LINE" "$CONTENT" "$HINTS" >> $GITHUB_STEP_SUMMARY | |
| else | |
| printf '%s\n matched: %s\n → review flagged RTL-related class usage\n' "$FILE_LINE" "$CONTENT" >> $GITHUB_STEP_SUMMARY | |
| fi |
| steps: | ||
| - name: Checkout | ||
| uses: actions/checkout@v4 | ||
|
|
There was a problem hiding this comment.
This workflow uses actions/checkout@v4 unpinned. Elsewhere in this repo the checkout action is pinned to a commit SHA (see .github/workflows/ci.yml), which is safer against supply-chain changes. Consider pinning actions/checkout (and actions/github-script) to SHAs here as well for consistency and security.
| | `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.
The repo already contains a ci/ directory (and CI validation checks for it). Adding a separate CI/ directory can cause path conflicts on case-insensitive filesystems (macOS/Windows) and is easy to confuse in docs. Consider consolidating under a single casing (e.g., keep everything under ci/ or move the workflow to .github/workflows/).
| | `CI/rtl-check.yml` | GitHub Actions: blocks PRs containing physical direction classes | | |
| | `ci/rtl-check.yml` | GitHub Actions: blocks PRs containing physical direction classes | |
Automated: chore/merge-remote-bootstrap