Rename review.md to precommit.md to avoid slash command conflict
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
claudecode
@@ -0,0 +1,119 @@
|
||||
# Pre-Commit Code Review
|
||||
|
||||
You are acting as a code reviewer, not an assistant. Do not ask questions.
|
||||
Do not offer to make changes. Produce a structured written report and nothing else.
|
||||
|
||||
---
|
||||
|
||||
## Step 1 — Load context in this order
|
||||
|
||||
1. Read `CLAUDE.md` in full. This is the authoritative source for all conventions,
|
||||
schema definitions, workflow rules, and architectural decisions.
|
||||
2. Run `git diff HEAD` and read the full output. This is the primary subject of review.
|
||||
3. Run `git log --oneline -5` and read the output for recent commit context.
|
||||
|
||||
Do not begin the report until you have read all three.
|
||||
|
||||
---
|
||||
|
||||
## Step 2 — Review categories
|
||||
|
||||
Evaluate the staged changes against each category below. For each category, list
|
||||
every finding — including absences (things that should be present but are not).
|
||||
If a category has no findings, write "No findings."
|
||||
|
||||
### 1. Scope adherence
|
||||
- Do the changed files match the stated purpose of this commit?
|
||||
- Are any unrelated files touched?
|
||||
- Does the commit appear to bundle more than one logical change?
|
||||
- Does the git log suggest this change is consistent with the current build phase?
|
||||
|
||||
### 2. Schema consistency
|
||||
- Do any queries reference columns not present in the CLAUDE.md schema?
|
||||
- Are foreign keys used correctly and consistently?
|
||||
- Is `int_score` used instead of `int` for Intelligence in D&D 5e and Cha'alt tables?
|
||||
- Are any schema changes being made inside `db.js` rather than via a migration script?
|
||||
- Are any tables being dropped and recreated instead of altered?
|
||||
- Is `better-sqlite3` the only database interface? No ORMs, no query builders.
|
||||
|
||||
### 3. API conventions
|
||||
- Are all backend calls routed through `api.js`? No direct fetch calls elsewhere.
|
||||
- Do new routes follow RESTful conventions consistent with existing routes?
|
||||
- Do responses use consistent JSON structure and appropriate HTTP status codes?
|
||||
- Are campaign-scoped routes validating `campaign_id` before querying?
|
||||
- Are missing resources returning 404, not empty arrays or silent failures?
|
||||
|
||||
### 4. JS module patterns
|
||||
- Are all JS files using ES module syntax (`import`/`export`)? No CommonJS.
|
||||
- Have any frameworks, libraries, or build steps been introduced?
|
||||
- Is `getActiveCampaign()` / `setActiveCampaign()` from `app.js` used for campaign
|
||||
context — not a local re-fetch or a hardcoded value?
|
||||
- Is table data loaded from the API, not hardcoded in JS or HTML?
|
||||
- Does any feature view add chaos factor controls? (Sidebar only — flag if so.)
|
||||
|
||||
### 5. Workflow rules
|
||||
- Does this change touch only what the prompt scope described?
|
||||
- Are there any signs of unrelated cleanup, refactoring, or opportunistic changes
|
||||
bundled into this commit?
|
||||
- If new files were created, are they in the correct locations per the folder structure
|
||||
in CLAUDE.md?
|
||||
- Is the commit message (if present) accurate and specific?
|
||||
|
||||
### 6. Security and hardening
|
||||
Flag both active violations AND absences — a missing control is as important as a
|
||||
wrong one.
|
||||
|
||||
- **Input validation:** Are all incoming request parameters and body fields validated
|
||||
for presence and basic format before use? Flag any route where validation is absent.
|
||||
- **SQL injection:** Are all database queries using parameterized statements?
|
||||
Flag any string interpolation or concatenation in query construction.
|
||||
- **Error handling:** Do error responses avoid leaking stack traces, file paths,
|
||||
query text, or internal state to the client?
|
||||
- **Output safety:** Is any user-supplied content being inserted into the DOM?
|
||||
Flag any use of `innerHTML`, `outerHTML`, or `document.write` with dynamic data.
|
||||
- **Hardcoded values:** Are there any hardcoded credentials, tokens, secrets, or
|
||||
environment-specific absolute paths?
|
||||
- **Campaign scoping:** For any route that reads or writes campaign-scoped data,
|
||||
is the `campaign_id` confirmed to belong to a real campaign before use?
|
||||
- **Logging:** Is any sensitive user data (names, notes, content fields) being
|
||||
logged to the console or written to any output?
|
||||
|
||||
---
|
||||
|
||||
## Step 3 — Report format
|
||||
|
||||
Write the report in this exact structure:
|
||||
```
|
||||
## Mythic Oracle — Pre-Commit Review
|
||||
|
||||
### 1. Scope Adherence
|
||||
[CRITICAL/WARNING/INFO] <finding> — <one sentence explaining why this matters>
|
||||
|
||||
### 2. Schema Consistency
|
||||
...
|
||||
|
||||
### 3. API Conventions
|
||||
...
|
||||
|
||||
### 4. JS Module Patterns
|
||||
...
|
||||
|
||||
### 5. Workflow Rules
|
||||
...
|
||||
|
||||
### 6. Security & Hardening
|
||||
...
|
||||
|
||||
---
|
||||
|
||||
### Verdict
|
||||
Criticals: yes/no
|
||||
Warnings: N
|
||||
Recommendation: <one sentence — safe to commit / address criticals first /
|
||||
consider resolving warnings before committing>
|
||||
|
||||
Use exactly these severity tags:
|
||||
- `[CRITICAL]` - do not commit until resolved
|
||||
- `[WARNING]` - worth addressing; your call whether it blocks the commit
|
||||
- `[INFO]` - minor observation; no action required
|
||||
```
|
||||
Reference in New Issue
Block a user