Standardize skill scaffolding and fix docs references#3118
Standardize skill scaffolding and fix docs references#3118ArnavBallinCode wants to merge 1 commit intofossasia:devfrom
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @ArnavBallinCode, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
There was a problem hiding this comment.
Pull request overview
Follow-up to #2767 that standardizes the .agents/skills/ scaffolding (SKILL.md + supporting subfolders) and updates documentation to consistently reference SKILL.md and the intended Docker deployment architecture.
Changes:
- Standardized skills layout across existing skills and added new skills (
frontend-ui,pull-request-workflow) with starter scripts/references/assets/tests. - Updated agent/docs indices and architecture navigation to be explicitly “selected/non-exhaustive” and to include key missing modules.
- Added a repo tool to validate skill directory structure.
Reviewed changes
Copilot reviewed 49 out of 49 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
tools/validate_skill_layout.sh |
Adds a validator enforcing required skill folders/files. |
agents.md |
Updates skills index to SKILL.md, adds new skills, and documents the intended skill layout. |
.github/instructions/dockerfile.instructions.md |
Updates production Compose guidance and clarifies host-level Nginx architecture. |
.agents/skills/README.md |
Introduces skills directory governance and validation guidance. |
.agents/context/architecture.md |
Expands the “selected modules” architecture map and clarifies it’s non-exhaustive. |
.agents/skills/repo-navigation/SKILL.md |
Updates repo navigation guidance with additional core modules and “non-exhaustive” framing. |
.agents/skills/repo-navigation/scripts/list_core_paths.sh |
Adds a helper script to print common repo paths for agents. |
.agents/skills/repo-navigation/references/navigation-guide.md |
Adds a reference guide for quick path discovery and guardrails. |
.agents/skills/repo-navigation/assets/path-selection-template.md |
Adds a template for documenting path selection decisions. |
.agents/skills/repo-navigation/tests/navigation-scenarios.md |
Adds scenario docs for validating navigation decisions. |
.agents/skills/pull-request-workflow/SKILL.md |
Adds a new skill documenting PR workflow expectations. |
.agents/skills/pull-request-workflow/scripts/check_pr_scope.sh |
Adds a helper script intended to flag oversized change sets. |
.agents/skills/pull-request-workflow/references/pr-readiness-guide.md |
Adds a PR readiness checklist reference. |
.agents/skills/pull-request-workflow/assets/pr-description-template.md |
Adds a reusable PR description template. |
.agents/skills/pull-request-workflow/tests/review-response-scenarios.md |
Adds scenarios for responding to review feedback. |
.agents/skills/frontend-ui/SKILL.md |
Adds a new skill for UI work, including accessibility and responsiveness guardrails. |
.agents/skills/frontend-ui/scripts/check_frontend_paths.sh |
Adds a helper script to validate changed-file paths for frontend work. |
.agents/skills/frontend-ui/references/ui-checklist.md |
Adds a UI checklist reference (behavior/accessibility/responsiveness). |
.agents/skills/frontend-ui/assets/ui-pr-checklist-template.md |
Adds a reusable UI PR checklist template. |
.agents/skills/frontend-ui/tests/ui-scenarios.md |
Adds UI validation scenarios (forms, mobile, keyboard navigation). |
.agents/skills/documentation/scripts/check_doc_refs.sh |
Adds a script to verify commonly referenced docs exist. |
.agents/skills/documentation/references/documentation-style-guide.md |
Adds documentation style and consistency guidance. |
.agents/skills/documentation/assets/doc-change-template.md |
Adds a documentation change template. |
.agents/skills/documentation/tests/doc-update-scenarios.md |
Adds scenarios for doc updates and reference validation. |
.agents/skills/docker-deployment/SKILL.md |
Updates onboarding command guidance and clarifies host-level Nginx in production. |
.agents/skills/docker-deployment/scripts/check_compose_files.sh |
Adds a script to verify required compose/env files exist. |
.agents/skills/docker-deployment/references/docker-deploy-checklist.md |
Adds a deployment checklist reference. |
.agents/skills/docker-deployment/assets/deploy-rollback-template.md |
Adds a deployment/rollback planning template. |
.agents/skills/docker-deployment/tests/deployment-scenarios.md |
Adds deployment scenario docs. |
.agents/skills/django-run-migrations/scripts/check_pending_migrations.sh |
Adds a script to fail if migrations are pending. |
.agents/skills/django-run-migrations/references/migration-safety-guide.md |
Adds migration safety reference guidance. |
.agents/skills/django-run-migrations/assets/migration-review-template.md |
Adds a migration review template. |
.agents/skills/django-run-migrations/tests/migration-scenarios.md |
Adds migration scenario docs. |
.agents/skills/django-run-locally/scripts/local_smoke.sh |
Adds a minimal local Django smoke-check script. |
.agents/skills/django-run-locally/references/local-run-troubleshooting.md |
Adds troubleshooting guidance for local runs. |
.agents/skills/django-run-locally/assets/local-start-checklist.md |
Adds a local start checklist template. |
.agents/skills/django-run-locally/tests/local-startup-scenarios.md |
Adds local startup scenario docs. |
.agents/skills/django-create-model/scripts/check_model_location.sh |
Adds a helper script to validate model file placement. |
.agents/skills/django-create-model/references/model-change-checklist.md |
Adds a model-change checklist reference. |
.agents/skills/django-create-model/assets/model-change-template.md |
Adds a model-change documentation template. |
.agents/skills/django-create-model/tests/model-change-scenarios.md |
Adds model-change scenario docs. |
.agents/skills/django-create-api-endpoint/scripts/check_api_route_registration.sh |
Adds a helper script to sanity-check API + routing file inclusion. |
.agents/skills/django-create-api-endpoint/references/api-endpoint-checklist.md |
Adds an API endpoint checklist reference. |
.agents/skills/django-create-api-endpoint/assets/api-response-template.md |
Adds an API endpoint change template. |
.agents/skills/django-create-api-endpoint/tests/api-endpoint-scenarios.md |
Adds API endpoint scenario docs. |
.agents/skills/django-add-form/scripts/check_form_location.sh |
Adds a helper script to validate form file placement. |
.agents/skills/django-add-form/references/form-validation-patterns.md |
Adds form validation patterns reference. |
.agents/skills/django-add-form/assets/form-review-template.md |
Adds a form-change review template. |
.agents/skills/django-add-form/tests/form-validation-scenarios.md |
Adds form validation scenario docs. |
| # On the production server | ||
| cd /home/$USER/$DEPLOYMENT_NAME | ||
| docker compose -f docker-compose.yml up -d | ||
| # Use the production compose file from the deployment/ directory | ||
| docker compose -f deployment/docker-compose.yml up -d | ||
| ``` |
There was a problem hiding this comment.
The production command example changes to -f deployment/docker-compose.yml but the preceding cd /home/$USER/$DEPLOYMENT_NAME does not appear to be the repo root in the current deployment guide (DEPLOYMENT.md clones into an eventyay/ subdir and symlinks a compose file). As written, deployment/docker-compose.yml likely won’t exist from that working directory. Please update the example (and/or DEPLOYMENT.md) so the compose path is copy/paste runnable and consistent with the actual on-server directory layout.
| - Skills **summarize and reference** existing instruction files; they do not duplicate coding rules. | ||
| - When in doubt, defer to the coding standards in `.github/instructions/`. | ||
| - All product code lives under `app/eventyay/`; use `eventyay.*` imports. No newline at end of file | ||
| - All product code lives under `app/eventyay/`; use `eventyay.*` imports. | ||
| - New skills should follow this layout: `SKILL.md`, `scripts/`, `references/`, `assets/`, and optional `tests/`. No newline at end of file |
There was a problem hiding this comment.
agents.md says the tests/ folder is optional for new skills, but the new skill README calls tests/ required and tools/validate_skill_layout.sh enforces a non-empty tests/ directory. Please make these sources consistent (either require tests/ everywhere or adjust the validator/docs to treat it as optional).
| skills_root=".agents/skills" | ||
| required_dirs=(scripts references assets tests) | ||
|
|
There was a problem hiding this comment.
The validator currently hard-requires a non-empty tests/ directory for every skill (required_dirs includes tests). This conflicts with the updated guidance in agents.md that calls tests/ optional. Either remove tests from required_dirs (or make it conditional) or update the docs to state that tests/ is required.
| - `scripts/` (reusable executable helpers) | ||
| - `references/` (detailed docs loaded on demand) | ||
| - `assets/` (templates/checklists/snippets) | ||
| - `tests/` (scenario docs or test cases) |
There was a problem hiding this comment.
This doc states that every skill directory "should contain" tests/, which currently conflicts with agents.md calling tests/ optional. Please align the wording with the intended policy (and with the validator behavior).
| - `tests/` (scenario docs or test cases) | |
| - `tests/` (scenario docs or test cases, optional but recommended) |
| case "$file" in | ||
| app/eventyay/static/*|app/eventyay/webapp/*|app/eventyay/*/templates/*) | ||
| echo "OK: $file" |
There was a problem hiding this comment.
The case pattern for templates only matches files one level under .../templates/ (e.g. app/eventyay/control/templates/control/base.html will not match). This will produce false WARNs for most real template paths. Consider broadening the match (e.g. allow deeper subpaths) so nested template files are treated as OK.
| # Quick guardrail: show changed files and fail if there are too many. | ||
| # Usage: ./check_pr_scope.sh [max_files] | ||
|
|
||
| max_files="${1:-40}" | ||
| count="$( (git diff --name-only HEAD~1..HEAD 2>/dev/null || true) | wc -l | tr -d ' ' )" | ||
|
|
||
| if [[ "$count" -eq 0 ]]; then | ||
| echo "No changed files detected in HEAD~1..HEAD." | ||
| exit 0 | ||
| fi | ||
|
|
||
| echo "Changed files in last commit: $count" | ||
| if [[ "$count" -gt "$max_files" ]]; then |
There was a problem hiding this comment.
This script claims to "show changed files" but only prints a count, and it only inspects HEAD~1..HEAD (last commit) rather than the full branch/PR diff. That can misleadingly report 0 changes on a single-commit branch root, or ignore earlier commits. Consider either updating the wording to reflect the actual behavior or changing the diff range/output to match the intent (e.g. list file names and compare against the merge base).
There was a problem hiding this comment.
i dont think we should use scripts
There was a problem hiding this comment.
why not @Saksham-Sirohi
It is the official method of writing skills , I know they aren't completely developed right now but scaffold
Follow up to #2767
What This PR Does
What This PR Adds
New skills:
Standardized folder structure for all skills:
Starter content across older skills in each of the new subfolders:
Skill governance improvements:
Validation