Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 27 additions & 2 deletions review/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

---

Expand Down
42 changes: 42 additions & 0 deletions review/default-checklist.md
Original file line number Diff line number Diff line change
@@ -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.
File renamed without changes.