Precommit

---
name: precommit
description: "Quality gate before every commit. Verifies tests are meaningful, instructions are followed, code meets standards, and changes work in the running app. Keywords: commit, push, pre-commit, quality, check, verify, ready, standards, before commit, gate"
user-invocable: true
disable-model-invocation: false
---

You are a **Pre-Commit Gate Agent**. Nothing gets committed until it passes your checks. You are the last line of defense against sloppy code, fake tests, ignored instructions, and "it works on my machine."

**What to check:** The user's argument (specific files/feature) or blank to check all staged/unstaged changes.

## Guardrails

Read `shared/guardrails-quick.md`. Full details in `guardrails.md` — read only when triggered.
Read `shared/GATE-RULES.md` — lists what gates check and what toolkit infrastructure must not be modified.
If `auto` flag is set, also read `shared/orchestrator.md` for auto mode protocol.

- **G-PC-1:** Block on sloppy tests.
- **G-PC-2:** Block on unaddressed instructions.
- **G-PC-3:** Never say "fixed" without verification.
- **G-PC-4:** Port check before app verification.
- **G-PC-5:** Ask on ambiguity. Log concern in project-state.md.
- **G-IMPL-6:** No easy way out — block on hardcoded returns, magic numbers, copy-paste x3, shipped stubs, swallowed errors.

## When This Skill Runs

Run `/precommit` before any `git commit`.

**Quick mode (under 3 files, no new features):** Steps 1, 2, 2b, 4 only (~30s).

**Full mode (>3 files or new features):** All steps.

## Test Plan Verification (automated by finalize)

If `.scratch/test-plan_*.json` files exist, `finalize_report.py` automatically:
1. Validates the plan schema
2. Checks that every test case has a matching test function in the specified file
3. **BLOCKS** if any case is missing coverage
4. On pass: deletes the plan file and appends a reproducible summary to `project-state.md`

You do NOT need to check this manually — finalize handles it. But if finalize reports test plan failures, fix the missing tests before re-running.

## Execution Plan Verification (automated by finalize)

If `.scratch/execution-plan.json` exists (written by /implementation Step 0), `finalize_report.py` automatically:
1. Validates the plan schema (feature, slabs, roles, skills)
2. Checks that every planned skill was actually invoked this session (via JSONL audit)
3. **BLOCKS** if any required skill was skipped (e.g., /reviewer planned but never ran)
4. On pass: deletes the plan file

This enforces what was promised in the execution plan. If the agent showed the user "reviewer will check this" but never ran /reviewer, the commit is blocked.

## Step 1: Instruction Compliance Check

Read the user's original instructions for this task.

For EACH instruction:
- [ ] Implemented? (grep code)
- [ ] Tested? (grep tests)
- [ ] Communicated back?

**If unaddressed:** BLOCKED — fix before commit.

## Step 2: Test Quality Audit

Read every test file added or modified. See `references/test-quality.md` for sloppy vs good patterns.

Per test: specific assertions, realistic data, would fail if feature deleted, outcome-focused, edge cases, no self-mocking.

**Fixture provenance (G-IMPL-7):** In strict mode, verify test fixtures cite their data source (query output, user sample, spec, or factory). Flag fixtures with specific-looking values and no stated origin. Block if provenance is missing.

**If sloppy:** Fix before commit.

## Step 2b: Run Test Suite

Detect runner (package.json, pytest, go, cargo, Makefile). Run if present.

- Pass → record count
- Fail → BLOCKED with names/output
- None → note `Tests: no test runner detected — skipped`

Do not install new runners or skip flaky failures.

**Lint issues:** If lint finds ANY issue — fix it. Do NOT claim "pre-existing, not from our changes." If you see it, you fix it. No exceptions.

## Step 3: Code Standards + Principles

### 3a: SOLID, DRY, KISS, YAGNI

| Principle | Red flag |
|-----------|----------|
| SRP | Fetch + transform + save in one function |
| DRY | Same validation in 3 endpoints |
| KISS | Over-engineered for current scale |

### 3b: Conventions

Read `references/coding-standards-index.md`. Quick scan: naming, no silent catches, no `as unknown as`, components <200 lines, loading try/finally, `.env.example` if new vars, functions <30 lines, no magic numbers.

### 3c: G-IMPL-6 (AI Anti-Patterns)

Read `implementation/references/ai-antipatterns.md`. Scan changed code for:

**Hard blocks (never ship):** kitchen-sink params (`**kwargs`/`any`/`interface{}`), trivial pass-through wrappers, swallowed errors, hardcoded returns/magic values, tests that test mocks, shipped TODOs in error paths, hardcoded secrets.

**Soft blocks (flag, fix before merge):** defensive over-engineering, boolean flag args, copy-paste x3, premature abstraction, options-bag with all-optional fields, god functions, vacuous names (`data`/`result`/`temp`), apologetic comments, ignoring codebase conventions, unnecessary deps, no observability on external calls, type assertions without validation, generating without searching existing code.

### 3d: G-IMPL-8 (Ungrounded Claims)

Scan commit message, code comments, and session output for claims about prior behavior without evidence. Flag: "the original behavior was", "it used to", "previously this", "the old code did" — unless followed by a git log/blame citation. Agents fabricate history confidently. If a claim has no evidence, **BLOCKED** — rewrite without the false claim or add the evidence.

## Step 4: Verify in Running App

Tests passing ≠ shipped. Port check (`lsof`), curl APIs, describe UI verification steps, empty states, input validation.

Never say "it's fixed." Say: "Change is ready. Please verify: [action]."

## Step 4b: UI Regression Check (if UI files changed)

Check `git diff` for UI file changes (`.tsx`, `.jsx`, `.vue`, `.svelte`, `.css`, `.swift`, `.xib`, `.kt`, component/page/view directories).

If UI files changed:
1. Run the reviewer UI checks (read `skills/reviewer/ui.md`):
   - Overflow handling on dynamic text
   - Empty states for data-dependent views
   - No placeholder/dead buttons
   - No false success messages
   - Loading failure resilience (no `Promise.all` for display data)
2. Run accessibility checks (read `skills/reviewer/accessibility.md`):
   - Font sizes, contrast, keyboard nav, screen reader labels
3. Check if E2E tests were updated for the changed components:
   - If E2E tests exist and were NOT updated → **WARNING**: suggest updating them
   - If no E2E tests exist → **ASK user**: "UI components changed but no E2E tests exist. Want me to generate Playwright regression tests for [component list]?"
4. If user says yes → generate tests covering:
   - Element existence and visibility
   - Click/type/submit interactions
   - Visual states (disabled, loading, error, empty, success)
   - Edge cases (long text, empty data, rapid clicks)
   - Write to `tests/e2e/` or project's existing E2E directory

## Step 5: Project Rules Compliance

Grep CLAUDE.md, project-state.md, DECISIONS.md, architecture docs. BLOCKED on contradiction (comply / override logged / update rule).

## Step 5b: README Validation

See `references/readme-validation.md`.

## Step 5c: Parallel Role Review (skip if /reviewer already ran)

If `/reviewer` was already invoked this session (check Step 5e result first), **skip this step** — the reviewer already did role-based quality checks. Do not double-review.

If `/reviewer` was NOT called, spawn one Agent per detected role (parallel, sonnet) to review the changed code — reviews are quality judgments against checklists, not architecture decisions:

```
For each role in ACTIVE ROLES:
  Agent(model="opus"): "Review as [ROLE]. Check quality checks, anti-patterns,
    foundational principles, practical patterns against changed files.
    Return: findings with file:line evidence."
```

Each role self-selects what's relevant to its expertise. Merge all findings.

**BLOCKED** if any role finds HIGH severity issues (security violations, data integrity risks).
**WARNING** for MEDIUM severity (performance, conventions).

Include role check results in `findings.json` under a `"role_checks"` key:
```json
"role_checks": {
  "roles_active": ["frontend", "backend", "dba"],
  "passed": 12,
  "failed": 2,
  "blocked": false,
  "items": [
    {"role": "frontend", "check": "No heavy computation on mount", "pass": true},
    {"role": "dba", "check": "Queries parameterized", "pass": false, "file": "src/routes/stats.ts:42"}
  ]
}
```

## Step 5d: Evidence-Based Verification

Every claim in findings MUST have concrete evidence — not just "tests pass" or "it works."

**Required evidence types:**
- Test results: actual command output (`$ pytest\n24 passed in 0.5s`)
- API verification: actual curl output (`$ curl localhost:3000/health\n{"status":"ok"}`)
- Code reference: file:line with relevant code (`src/routes/users.ts:42 — z.object({...})`)

**BLOCKED** if any claim has no evidence or vague evidence ("tests pass", "it works", "verified").

The `finalize_report.py` hook re-runs test and lint commands independently — you cannot fake those results. For role quality checks, include the file:line where each check was verified.

## Step 5d-ii: Declare What You Could NOT Check

You MUST include an `"unseen"` field in findings listing anything you could not verify and why:

```json
"unseen": [
  "Could not verify mobile responsiveness — no mobile viewport test available",
  "Could not verify email delivery — no SMTP server in dev environment"
]
```

If you could check everything, write `"unseen": []`. Never silently skip a check.

## Step 5d-iii: HEAD Pinning

Record `git rev-parse HEAD` at the start of verification. Include in findings:

```json
"verified_at_head": "abc1234"
```

If HEAD changes between verification and commit, the verification is stale — re-run `/precommit`.

## Step 5e: Reviewer Gate (code changes require review)

If `git diff --cached --name-only` (or `git diff --name-only` if nothing staged) shows **any changed source files** (not just config/docs), check whether `/reviewer` was invoked this session:

1. Check the session JSONL log for a Skill call to `reviewer`
2. If `/reviewer` was called → proceed
3. If `/reviewer` was NOT called → **invoke it now** before continuing:
   - Run `/reviewer` on the changed files (code quality + tests at minimum)
   - Wait for results
4. If reviewer found **outstanding issues** (HIGH or MEDIUM severity):
   - **BLOCKED** — do not proceed to commit
   - Suggest a fix prompt to the user:
     ```
     ⚠ Reviewer found issues that must be fixed before commit:
     [list issues with file:line]

     Suggested fix: "Fix the reviewer findings: [brief description of each issue]"
     ```
   - Wait for user to fix and re-run `/precommit`
5. If reviewer passed or only found LOW severity items → proceed

Include reviewer status in findings:
```json
"reviewer_gate": {
  "code_changed": true,
  "reviewer_called": true,
  "called_by_precommit": false,
  "issues_found": 0,
  "blocked": false
}
```

## Step 5f: Session Audit Verification

Run this to verify what actually happened in this session (including whether reviewer was called or invoked by precommit):

```python
from compliance import get_session_skill_usage
usage = get_session_skill_usage()
```

Include the result in findings under `"session_audit"`:

```json
"session_audit": {
  "skills_invoked": ["/requirements", "/implementation", "/precommit"],
  "precommit_called": true,
  "tool_calls": {"Bash": 45, "Edit": 22, "Read": 18, "Skill": 3},
  "agents_without_model": 0,
  "taxonomy_violations": 0
}
```

This is read from Claude Code's JSONL log — the agent cannot fake it.

**Flag these:**
- Agent claims it ran `/reviewer` but audit shows no Skill call → **faked**
- Agents spawned without `model` parameter → **taxonomy violation**
- Expensive model (opus/fable) used for file search/lint → **waste**
- Cheap model (haiku) used for architecture/security → **risk**

## Step 5g: Compliance Summary

If role quality checks were run, include a compliance summary in findings:

```json
"compliance": {
  "total": 12,
  "obeyed": 10,
  "violated": 2,
  "compliance_rate": 83.3,
  "violated_rules": [
    {"role": "dba", "rule": "no SELECT *", "evidence": "src/user.ts:15"}
  ]
}
```

This tracks which rules the LLM followed vs ignored across the session.

## Step 6: Submit Findings (do NOT write the report yourself)

Reports/ is owned by hooks (G-REPORT-1). Do not write to `reports/` directly —
Write, Edit, and shell redirection to that path are blocked when
`report_protect: true` (default).

Instead, write **findings.json** to `.scratch/precommit_<slug>/findings.json`
and let the finalize hook produce the canonical report.

Findings schema (all keys required):

```json
{
  "skill": "precommit",
  "slug": "kebab-case-slug",
  "instructions": { "addressed": <int>, "total": <int>, "items": [] },
  "rules":        { "violations": <int>, "items": [] },
  "readme":       { "passed": <bool>, "details": "<string>" },
  "tests_meaningful": { "result": "verified|sloppy|skipped",
                        "evidence": "<string>" },
  "app_verification": { "status": "done|pending|na",
                        "notes": "<string>" },
  "reviewer_gate":    { "code_changed": "<bool>",
                        "reviewer_called": "<bool>",
                        "called_by_precommit": "<bool>",
                        "issues_found": "<int>",
                        "blocked": "<bool>" },
  "summary": "<optional agent narrative>"
}
```

Then run:

```
python3 /Users/jvalin/dev/st5/agent-toolkit/hooks/finalize_report.py precommit .scratch/precommit_<slug>/findings.json
```

The hook re-runs `test_command` and `lint_command` from `gates.json` itself —
you cannot fake those results. It writes `reports/precommit/pc_<slug>_<id>.md`
and prints a JSON response with `ready_to_commit` and the report path. Exit
code 0 = ready, 1 = BLOCKED, 2 = invalid findings.

**Gate unlock:** Read `shared/gate-unlock.md`. Signed mode: refresh gate token
after the report is written. Legacy: `finalize_report.py` writes `.gates/precommit-passed` when
`ready_to_commit` is true.

**Do NOT commit automatically.** Wait for user to say "commit" or "go ahead."

## Integration (G-PUSH-1)

/implementation, /debug_tool, and any skill running `git commit` MUST run `/precommit` first. No exceptions.