From 8dd70017ed3a359f8b5adb85454717ca07f4ef6f Mon Sep 17 00:00:00 2001 From: Tyrone Robb Date: Sat, 14 Mar 2026 14:07:38 +0000 Subject: [PATCH] feat(review): implement fallback chain for review checklists --- review/SKILL.md | 29 +++++++++++++- review/default-checklist.md | 42 +++++++++++++++++++++ review/{checklist.md => rails-checklist.md} | 0 3 files changed, 69 insertions(+), 2 deletions(-) create mode 100644 review/default-checklist.md rename review/{checklist.md => rails-checklist.md} (100%) diff --git a/review/SKILL.md b/review/SKILL.md index 217e93d..69373a9 100644 --- a/review/SKILL.md +++ b/review/SKILL.md @@ -39,9 +39,34 @@ You are running the `/review` workflow. Analyze the current branch's diff agains ## Step 2: Read the checklist -Read `.claude/skills/review/checklist.md`. +You must find and read the appropriate review checklist based on this fallback chain: -**If the file cannot be read, STOP and report the error.** Do not proceed without the checklist. +1. **Configured path**: Check `.gstack.json` for `"reviewChecklist"`. If present and the file exists, read it. +2. **Custom project path**: If the file `.claude/skills/review/checklist.md` exists in the local project, read it. +3. **Bundled default fallback**: If neither exists, read the universal default checklist at `~/.claude/skills/gstack/review/default-checklist.md`. + +```bash +CHECKLIST="" +if [ -f .gstack.json ]; then + CONFIG_CHECKLIST=$(grep '"reviewChecklist"' .gstack.json | cut -d'"' -f4) + if [ -n "$CONFIG_CHECKLIST" ] && [ -f "$CONFIG_CHECKLIST" ]; then + CHECKLIST="$CONFIG_CHECKLIST" + fi +fi + +if [ -z "$CHECKLIST" ] && [ -f ".claude/skills/review/checklist.md" ]; then + CHECKLIST=".claude/skills/review/checklist.md" +fi + +if [ -z "$CHECKLIST" ]; then + CHECKLIST="~/.claude/skills/gstack/review/default-checklist.md" +fi + +echo "Using review checklist: $CHECKLIST" +cat $(eval echo "$CHECKLIST") +``` + +**If the chosen file cannot be read, STOP and report the error.** Do not proceed without the checklist. --- diff --git a/review/default-checklist.md b/review/default-checklist.md new file mode 100644 index 0000000..1ebf779 --- /dev/null +++ b/review/default-checklist.md @@ -0,0 +1,42 @@ +# Pre-Landing Review Checklist (Universal) + +## PASS 1: CRITICAL +If any of these are violated, it is a CRITICAL issue. + +### SQL & Data Safety +- No raw SQL literals constructed from variables (SQL injection). +- No unescaped user input passed to database queries. +- Mass assignment must be explicitly permitted/filtered. +- No sensitive data logged in plaintext. + +### LLM Output Trust Boundary +- Never parse LLM text output as code/JSON without strict sanitization. +- Always use structured output/tool use when machine parsing is required. +- Provide clear fallbacks for when the LLM hallucinates or returns invalid structures. + +### Secrets & Credentials +- No hardcoded API keys, tokens, or passwords in source code. +- Credentials must be loaded from environment variables or secure stores. + +## PASS 2: INFORMATIONAL +If any of these are violated, it is an INFORMATIONAL issue. + +### Error Handling & Edge Cases +- All external API calls must handle timeouts and network failures. +- Async operations must catch and log unhandled rejections. +- Promises must be awaited or explicitly returned. + +### Dead Code & Consistency +- No commented-out blocks of code left behind. +- Remove unused variables and imports. +- Follow the established naming conventions of the surrounding code. + +### Security +- No path traversal vulnerabilities when reading/writing files based on user input. +- Cross-Site Scripting (XSS) protections must be active when rendering user input. + +## DO NOT flag (Common False Positives) +- Debug logging that does not contain PII or secrets. +- Missing tests (unless the PR explicitly claims to add them). +- Subjective style preferences (e.g., tabs vs spaces) unless violating a linter. +- Known technical debt that is outside the scope of the current diff. diff --git a/review/checklist.md b/review/rails-checklist.md similarity index 100% rename from review/checklist.md rename to review/rails-checklist.md