feat: CI validation for SKILL.md, CONTRIBUTING guide, repo metadata#5
Conversation
Adds a "Validate SKILL.md files" step to the existing CI workflow that checks every SKILL.md has YAML frontmatter and a SECURITY GUARDRAIL comment (substring match, compatible with all existing skill files). Adds CONTRIBUTING.md explaining how to add mappings, fix examples, and create new skill files including the CI requirements. Co-Authored-By: Claude Sonnet 4.6 <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 52 minutes and 41 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 (2)
✨ 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: 894b614a15
ℹ️ 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".
| if not content.startswith('---'): | ||
| errors.append(f'{skill_file}: missing YAML frontmatter') | ||
| elif GUARDRAIL_PREFIX not in content: | ||
| errors.append(f'{skill_file}: missing SECURITY GUARDRAIL comment') |
There was a problem hiding this comment.
Validate frontmatter block instead of leading marker
In the docs-and-structure job’s Validate SKILL.md files step, content.startswith('---') only checks the first three characters and does not confirm a valid YAML frontmatter block is present. A malformed SKILL.md (for example, no closing frontmatter delimiter or invalid header content) will still pass as long as it contains the guardrail substring, which defeats the CI check’s stated purpose and can let broken skill metadata merge undetected.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR adds contributor documentation and CI validation to ensure all SKILL.md files include required structural elements (frontmatter + security guardrail), aligning repo contributions with the “skills” workflow used by AI coding assistants.
Changes:
- Added
CONTRIBUTING.mdwith repo layout, contribution workflow, skill-file requirements, and a local validation snippet. - Extended
.github/workflows/ci.ymlwith a step to validateSKILL.mdfiles for frontmatter +<!-- SECURITY GUARDRAIL:presence.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
CONTRIBUTING.md |
New contributor guide documenting repo structure, skill requirements, and local validation instructions. |
.github/workflows/ci.yml |
Adds CI enforcement for basic SKILL.md structure (frontmatter prefix + guardrail substring check). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| 1. **Start with YAML frontmatter** (`---` block) containing at least `name`, `description`, and `triggers`. | ||
| 2. **Include the security guardrail comment** immediately after the frontmatter closing `---`: | ||
| ``` | ||
| <!-- SECURITY GUARDRAIL: ... --> | ||
| ``` | ||
| The comment text should describe what the skill does and does not do. See `skills/rtl-fix/SKILL.md` for a concrete example. | ||
| 3. **List specific triggers** — phrases that should cause an AI assistant to load this skill automatically (e.g., `"rtl fix"`, `"code review"`, `"tsx file edit"`). | ||
|
|
||
| CI will fail if either requirement is missing. |
There was a problem hiding this comment.
This section says SKILL frontmatter must include at least name, description, and triggers, and that the SECURITY GUARDRAIL comment must appear immediately after the closing ---. The CI validator added in this PR only checks that the file starts with --- and contains the guardrail substring somewhere, so CI will not actually enforce the stricter requirements described here. Either tighten the CI validator to match these rules (e.g., parse frontmatter + check guardrail placement), or relax the doc language to match what CI enforces.
|
|
||
| ## Pull request checklist | ||
|
|
||
| - [ ] CI passes locally (run `python3 .github/validate-skills.py` if available, or check the CI log) |
There was a problem hiding this comment.
The checklist suggests running python3 .github/validate-skills.py, but there is no .github/validate-skills.py in this repository. Consider either adding that script (so contributors can run the same validation locally), or updating this checklist item to reference the inline snippet below (or another existing command).
| - [ ] CI passes locally (run `python3 .github/validate-skills.py` if available, or check the CI log) | |
| - [ ] CI passes locally (run the inline validation snippet in [Running CI validation locally](#running-ci-validation-locally), or check the CI log) |
| GUARDRAIL_PREFIX = '<!-- SECURITY GUARDRAIL:' | ||
| errors = [] | ||
| skill_files = [f for f in Path('.').rglob('SKILL.md') if '.git' not in f.parts] | ||
| for skill_file in skill_files: | ||
| content = skill_file.read_text(encoding='utf-8') | ||
| if not content.startswith('---'): | ||
| errors.append(f'{skill_file}: missing YAML frontmatter') | ||
| elif GUARDRAIL_PREFIX not in content: |
There was a problem hiding this comment.
The validator only checks content.startswith('---'), which can pass even if there is no closing frontmatter delimiter or if the frontmatter is malformed. If the intent is to enforce valid YAML frontmatter, consider locating the second --- and parsing that block with yaml.safe_load (and failing if it can’t be parsed).
| GUARDRAIL_PREFIX = '<!-- SECURITY GUARDRAIL:' | |
| errors = [] | |
| skill_files = [f for f in Path('.').rglob('SKILL.md') if '.git' not in f.parts] | |
| for skill_file in skill_files: | |
| content = skill_file.read_text(encoding='utf-8') | |
| if not content.startswith('---'): | |
| errors.append(f'{skill_file}: missing YAML frontmatter') | |
| elif GUARDRAIL_PREFIX not in content: | |
| import yaml | |
| GUARDRAIL_PREFIX = '<!-- SECURITY GUARDRAIL:' | |
| errors = [] | |
| skill_files = [f for f in Path('.').rglob('SKILL.md') if '.git' not in f.parts] | |
| for skill_file in skill_files: | |
| content = skill_file.read_text(encoding='utf-8') | |
| if not content.startswith('---\n'): | |
| errors.append(f'{skill_file}: missing YAML frontmatter') | |
| continue | |
| closing_index = content.find('\n---\n', 4) | |
| if closing_index == -1: | |
| errors.append(f'{skill_file}: malformed YAML frontmatter') | |
| continue | |
| frontmatter = content[4:closing_index] | |
| try: | |
| yaml.safe_load(frontmatter) | |
| except yaml.YAMLError as exc: | |
| errors.append(f'{skill_file}: malformed YAML frontmatter ({exc})') | |
| continue | |
| if GUARDRAIL_PREFIX not in content: |
| - name: Validate SKILL.md files (frontmatter + guardrail) | ||
| run: | |
There was a problem hiding this comment.
The PR description/test plan mentions this validation step runs on ubuntu-latest, but this job is configured to run on the self-hosted pop-os runner. Please update the PR description/test plan (or the workflow) so they match, since it affects reproducibility of the checks.
| | `flutter/` | Flutter directional layout API patterns and migration notes | | ||
| | `nextjs/` | Next.js `dir` propagation, App Router, and i18n routing patterns | | ||
| | `biome/` | Biome lint policy rules that block physical-direction regressions | | ||
| | `ci/` | CI workflow snippets for RTL validation in real pipelines | |
There was a problem hiding this comment.
The repo also contains a top-level CI/ directory (e.g. CI/rtl-check.yml) that’s referenced from README, but it’s not listed in this directory layout table. Consider adding CI/ here (or consolidating CI/ vs ci/) to avoid contributor confusion.
| | `ci/` | CI workflow snippets for RTL validation in real pipelines | | |
| | `ci/` | CI workflow snippets for RTL validation in real pipelines | | |
| | `CI/` | Top-level CI workflow/config files referenced elsewhere in the repo (for example, `CI/rtl-check.yml`) | |
Summary
.github/workflows/ci.yml— checks that everySKILL.mdhas YAML frontmatter and a<!-- SECURITY GUARDRAIL:comment. Uses substring matching so it works with all three existing skill files (each has a custom guardrail message). Preserves the self-hosted runner, pinned checkout SHA, concurrency config, and all existing steps.CONTRIBUTING.md— explains directory layout, how to add mappings, fix examples, create new skill files (with explicit CI requirements), coding conventions, PR checklist, and a local validator snippet.main).rtl tailwind flutter nextjs css-logical-properties biome a11y i18n arabic hebrew ai-coding-assistant claude-code(already live onmain).Test plan
docs-and-structurejob targetsself-hosted, this is a new step within that same job)All 3 SKILL.md files valid)