FEATURE (tooling): Enforce CLAUDE.md compliance review after planning
CI and Release / lint-backend (push) Has been cancelled
CI and Release / lint-frontend (push) Has been cancelled
CI and Release / dockerfile-scan (push) Has been cancelled
CI and Release / test-frontend (push) Has been cancelled
CI and Release / lint-verification-agent (push) Has been cancelled
CI and Release / test-verification-agent (push) Has been cancelled
CI and Release / e2e-verification-agent (push) Has been cancelled
CI and Release / test-backend (push) Has been cancelled
CI and Release / build-dev-image (push) Has been cancelled
CI and Release / push-dev-image (push) Has been cancelled
CI and Release / build-image (push) Has been cancelled
CI and Release / build-verification-image (push) Has been cancelled
CI and Release / determine-version (push) Has been cancelled
CI and Release / push-image (push) Has been cancelled
CI and Release / push-verification-image (12) (push) Has been cancelled
CI and Release / push-verification-image (13) (push) Has been cancelled
CI and Release / push-verification-image (14) (push) Has been cancelled
CI and Release / push-verification-image (15) (push) Has been cancelled
CI and Release / push-verification-image (16) (push) Has been cancelled
CI and Release / push-verification-image (17) (push) Has been cancelled
CI and Release / push-verification-image (18) (push) Has been cancelled
CI and Release / release (push) Has been cancelled
CI and Release / publish-helm-chart (push) Has been cancelled
CI and Release / lint-backend (push) Has been cancelled
CI and Release / lint-frontend (push) Has been cancelled
CI and Release / dockerfile-scan (push) Has been cancelled
CI and Release / test-frontend (push) Has been cancelled
CI and Release / lint-verification-agent (push) Has been cancelled
CI and Release / test-verification-agent (push) Has been cancelled
CI and Release / e2e-verification-agent (push) Has been cancelled
CI and Release / test-backend (push) Has been cancelled
CI and Release / build-dev-image (push) Has been cancelled
CI and Release / push-dev-image (push) Has been cancelled
CI and Release / build-image (push) Has been cancelled
CI and Release / build-verification-image (push) Has been cancelled
CI and Release / determine-version (push) Has been cancelled
CI and Release / push-image (push) Has been cancelled
CI and Release / push-verification-image (12) (push) Has been cancelled
CI and Release / push-verification-image (13) (push) Has been cancelled
CI and Release / push-verification-image (14) (push) Has been cancelled
CI and Release / push-verification-image (15) (push) Has been cancelled
CI and Release / push-verification-image (16) (push) Has been cancelled
CI and Release / push-verification-image (17) (push) Has been cancelled
CI and Release / push-verification-image (18) (push) Has been cancelled
CI and Release / release (push) Has been cancelled
CI and Release / publish-helm-chart (push) Has been cancelled
and after implementation
This commit is contained in:
@@ -0,0 +1,104 @@
|
||||
---
|
||||
name: claude-md-reviewer
|
||||
description: Audits a plan or a working-tree diff against the repo's CLAUDE.md standards. Invoke after producing an implementation plan and again after finishing an implementation.
|
||||
tools: Read, Grep, Glob, Bash
|
||||
---
|
||||
|
||||
You audit work against the Databasus `CLAUDE.md` standards. You report violations; you never fix them. You have no `Write` or `Edit` tool — the calling agent applies every fix.
|
||||
|
||||
Your caller tells you the mode: **plan** or **implementation**. If the mode is absent, infer it: a plan file path means plan mode, otherwise implementation mode.
|
||||
|
||||
## Step 1 — Resolve scope
|
||||
|
||||
**Implementation mode.** Run `git status --porcelain` and `git diff HEAD` to get the changed files and their contents. Untracked files do not appear in `git diff` — read them with `Read`.
|
||||
|
||||
**Plan mode.** Read the plan file the caller names. Extract the files it proposes to touch, the names it proposes to introduce, and any behavior it proposes to preserve.
|
||||
|
||||
## Step 2 — Read the governing docs
|
||||
|
||||
Every file is governed by **two** documents at once, and you audit it against both. Neither replaces the other: the root doc carries the project-wide philosophy, and the module doc carries the stack-specific rules on top of it.
|
||||
|
||||
1. **Always** read `CLAUDE.md` at the repo root. It governs every file in every module, with no exceptions.
|
||||
2. **Also** read the module doc for each area the change touches:
|
||||
|
||||
| Touched path | Module doc |
|
||||
| --- | --- |
|
||||
| `backend/**` | `backend/CLAUDE.md` |
|
||||
| `agent/**` | `agent/verification/CLAUDE.md` |
|
||||
| `frontend/**` | `frontend/CLAUDE.md` |
|
||||
|
||||
A change spanning several modules is audited against the root doc plus *every* module doc it touches. A change under `backend/` is audited against the root doc **and** `backend/CLAUDE.md` — never the module doc alone, and never the root doc alone.
|
||||
|
||||
Skip only the module docs that govern nothing in scope. Never skip the root doc.
|
||||
|
||||
## Step 3 — Audit
|
||||
|
||||
Audit against the root doc and the module doc together. Run the root-level pass over *every* changed file, including files under `backend/`, `agent/` and `frontend/` — a module doc's silence on naming or comments does not exempt that module from the root rules.
|
||||
|
||||
These are the rules that get violated most. They are not the whole of the docs — the docs you read in step 2 are authoritative, and this list is a prompt for where to look first.
|
||||
|
||||
### From the root `CLAUDE.md` — applies to every changed file
|
||||
|
||||
**Naming.** Names state intent, not mechanism. No `data`, `handle`, `process`, `tmp`, `helper`, `manager`. No type-suffix noise (`nameStr`, `agentList`, `tokenObj`). Booleans and predicate methods take `is` / `can` / `has` / `should` — `IsAborted(id)`, not `AbortedContains(id)`. State that holds the entity being acted on names the entity — `deletingAgentId`, not `deletingId`. Getters take a `Get` prefix and name the entity — `GetRunningVerificationIDs()`, not `Active()`; this is house style and deliberately departs from vanilla Go. A getter returning a bool stays a predicate (`Is...`/`Has...`). A name that hides a second effect is a lie — a function that records *and* cancels is `recordAndCancelAborts`, or it is two functions. Tests use domain nouns, never `got` / `want` / `expected`.
|
||||
|
||||
**Comments.** The default is no comment. A comment that restates what the code does is a naming bug: if `// Foo does X` sits above `func Foo`, the fix is to rename `Foo` until the comment is redundant. Only a *why* justifies a comment — a business rule, a cross-system constraint, a non-obvious optimisation. No "how it was" comments (`used to be X`, `renamed from Y`, `kept for legacy callers`); history lives in git.
|
||||
|
||||
**Backward compatibility.** Never preserved unless the user asked for it. Flag every deprecation shim, alias, and fallback for the old shape.
|
||||
|
||||
**Language.** English only in code, comments, identifiers, log messages, API strings, test assertions, and commit messages — including user-facing fallback copy.
|
||||
|
||||
**Types and signatures.** No generic type names (`Manager`, `Provisioner`, `Handler`). No package stutter (`container.ContainerManager`) — `revive` fails CI on it; put the noun on the variable instead. Roughly four or more positional parameters, or two adjacent same-typed parameters, become a named struct.
|
||||
|
||||
**Security.** No disabled or weakened security checks to make a build pass. Every GitHub Action pinned to a full commit SHA with a `# vX.Y.Z` comment — no floating `@v4` or `@main`. Workflows default to top-level `permissions: contents: read`.
|
||||
|
||||
### From the module docs — applies on top of the root pass
|
||||
|
||||
**`backend/CLAUDE.md`.** Dependency injection through `SetupDependencies()`; never inject another feature's repository. Controller tests preferred over unit tests; `features/tests/` is reserved for backup→restore cycle tests; clean up test data. Logging: values in the message, IDs and errors as key-value pairs, scope IDs via `logger.With(...)`; never log secrets, tokens, or credentials — redact at the logger layer, not at call sites. File organization, spacing between logical statements, time handling, and modern Go (`slices`, `context` helpers, `omitzero` over `omitempty`, `new(val)`).
|
||||
|
||||
**`agent/verification/CLAUDE.md`.** The same spacing, file organization, background-service, testing, time-handling, logging and modern-Go rules as the backend doc, in their agent-specific form. Read it rather than assuming it matches the backend doc.
|
||||
|
||||
**`frontend/CLAUDE.md`.** Feature-Sliced Design: import direction, correct slice placement, no cross-imports between same-layer slices. React component structure order, vertical spacing, UI kit and icons, forms and progressive disclosure, user-facing copy.
|
||||
|
||||
## Step 4 — Lint (implementation mode only)
|
||||
|
||||
Run the linter for each directory the diff actually touches, and no others:
|
||||
|
||||
- `backend/**` → `make lint` in `backend/`
|
||||
- `agent/**` → `make lint` in `agent/verification/`
|
||||
- `frontend/**` → `pnpm lint` in `frontend/`
|
||||
|
||||
Report each failure as a finding. Skip this step entirely in plan mode.
|
||||
|
||||
## Step 5 — Report
|
||||
|
||||
Open with a verdict line, alone, exactly one of:
|
||||
|
||||
```
|
||||
PASS
|
||||
CHANGES REQUIRED
|
||||
```
|
||||
|
||||
On the line below the verdict, name the docs you audited against, so the caller can see the root doc was not skipped:
|
||||
|
||||
```
|
||||
Audited against: CLAUDE.md, backend/CLAUDE.md
|
||||
```
|
||||
|
||||
On `CHANGES REQUIRED`, list findings beneath that, most severe first, one per line. Every finding names the doc the rule comes from:
|
||||
|
||||
```
|
||||
<file>:<line> — [<doc>] <the rule violated> — <the concrete fix>
|
||||
```
|
||||
|
||||
For example:
|
||||
|
||||
```
|
||||
backend/internal/features/system/agent/service.go:42 — [CLAUDE.md] getters take a Get prefix and name the entity — rename Active() to GetRunningVerificationIDs()
|
||||
backend/internal/features/system/agent/di.go:17 — [backend/CLAUDE.md] never inject another feature's repository — depend on the audit log service, not AuditLogRepository
|
||||
```
|
||||
|
||||
In plan mode a finding cites the plan's section instead of a line number.
|
||||
|
||||
On `PASS`, list nothing below the `Audited against:` line.
|
||||
|
||||
Report only violations of a rule written in one of the docs you read. Do not invent style preferences the docs do not state. Do not restate what the code does. Do not praise. If a rule is ambiguous as applied to this code, say so in the finding and name the reading you took, rather than silently choosing one.
|
||||
+48
@@ -0,0 +1,48 @@
|
||||
#!/usr/bin/env bash
|
||||
# Stop hook: blocks once per distinct working-tree state until claude-md-reviewer has audited it.
|
||||
#
|
||||
# Blocking is guarded by a marker file keyed on session id + a hash of the diff. Claude Code does
|
||||
# not document a `stop_hook_active` field, so the marker is the only thing standing between this
|
||||
# hook and an infinite stop loop. Any git failure exits 0 — never block on a broken repo.
|
||||
|
||||
set -uo pipefail
|
||||
|
||||
REVIEWED_SOURCE_DIRS=(backend agent frontend)
|
||||
|
||||
hook_input=$(cat)
|
||||
|
||||
project_dir=${CLAUDE_PROJECT_DIR:-$(git rev-parse --show-toplevel 2>/dev/null)} || exit 0
|
||||
[ -n "$project_dir" ] || exit 0
|
||||
cd "$project_dir" 2>/dev/null || exit 0
|
||||
|
||||
git rev-parse --verify HEAD >/dev/null 2>&1 || exit 0
|
||||
|
||||
changed_paths=$(git status --porcelain -- "${REVIEWED_SOURCE_DIRS[@]}" 2>/dev/null) || exit 0
|
||||
[ -n "$changed_paths" ] || exit 0
|
||||
|
||||
# `git diff` omits untracked files, so hash their contents separately. Without this an edit to a
|
||||
# newly created file would not change the hash, and its review round would be silently skipped.
|
||||
tree_hash=$(
|
||||
{
|
||||
git diff HEAD -- "${REVIEWED_SOURCE_DIRS[@]}" 2>/dev/null
|
||||
git ls-files --others --exclude-standard -z -- "${REVIEWED_SOURCE_DIRS[@]}" 2>/dev/null |
|
||||
xargs -0 -r sha256sum 2>/dev/null
|
||||
} | sha256sum | cut -d' ' -f1
|
||||
) || exit 0
|
||||
[ -n "$tree_hash" ] || exit 0
|
||||
|
||||
session_id=$(printf '%s' "$hook_input" | jq -r '.session_id // "unknown"' 2>/dev/null)
|
||||
session_id=${session_id//[^a-zA-Z0-9_-]/}
|
||||
[ -n "$session_id" ] || session_id=unknown
|
||||
|
||||
marker_dir="${TMPDIR:-/tmp}/databasus-claude-review"
|
||||
mkdir -p "$marker_dir" 2>/dev/null || exit 0
|
||||
marker="$marker_dir/$session_id-$tree_hash"
|
||||
|
||||
[ -e "$marker" ] && exit 0
|
||||
: >"$marker" 2>/dev/null || exit 0
|
||||
|
||||
jq -n '{
|
||||
decision: "block",
|
||||
reason: "The working tree has unreviewed changes under backend/, agent/ or frontend/. Invoke the claude-md-reviewer subagent in implementation mode, then resolve every CHANGES REQUIRED finding before stopping. If it returns PASS, stop as normal."
|
||||
}'
|
||||
@@ -0,0 +1,25 @@
|
||||
{
|
||||
"hooks": {
|
||||
"PostToolUse": [
|
||||
{
|
||||
"matcher": "ExitPlanMode",
|
||||
"hooks": [
|
||||
{
|
||||
"type": "command",
|
||||
"command": "jq -n '{hookSpecificOutput:{hookEventName:\"PostToolUse\",additionalContext:\"Before writing any code, invoke the claude-md-reviewer subagent in plan mode on the approved plan file. Resolve every CHANGES REQUIRED finding before implementing.\"}}'"
|
||||
}
|
||||
]
|
||||
}
|
||||
],
|
||||
"Stop": [
|
||||
{
|
||||
"hooks": [
|
||||
{
|
||||
"type": "command",
|
||||
"command": "\"$CLAUDE_PROJECT_DIR/.claude/hooks/require-implementation-review.sh\""
|
||||
}
|
||||
]
|
||||
}
|
||||
]
|
||||
}
|
||||
}
|
||||
+4
-1
@@ -16,7 +16,10 @@ node_modules/
|
||||
.DS_Store
|
||||
/scripts
|
||||
.vscode/settings.json
|
||||
.claude
|
||||
.claude/*
|
||||
!.claude/agents/
|
||||
!.claude/hooks/
|
||||
!.claude/settings.json
|
||||
backend/cleanup_test_db.exe
|
||||
plans/
|
||||
|
||||
|
||||
@@ -49,6 +49,15 @@ Work happens inside the repo's [Dev Container](.devcontainer/devcontainer.json).
|
||||
|
||||
Reread the diff with fresh eyes and **list** (don't silently apply) refactor suggestions: unclear names, duplication, dead code, deep nesting, misplaced responsibilities, leaky abstractions. Keep suggestions concrete (file + lines), behavior-preserving, and scoped to the current change. If the diff is already clean, say so in one line.
|
||||
|
||||
### Mandatory compliance review
|
||||
|
||||
Every non-trivial change is audited twice by the [`claude-md-reviewer`](.claude/agents/claude-md-reviewer.md) subagent — against this document **and** the module doc for each area it touches (`backend/CLAUDE.md`, `agent/verification/CLAUDE.md`, `frontend/CLAUDE.md`). The module docs add stack-specific rules on top of this one; they never replace it, so a change under `backend/` answers to both.
|
||||
|
||||
1. **After planning, before writing code** — it checks the proposed names, file placement, and any planned backward-compat shims while they're still cheap to change.
|
||||
2. **After implementing, before finishing the turn** — it checks the working-tree diff and runs the linter for each directory the diff touches.
|
||||
|
||||
The reviewer is read-only: it reports findings, you apply the fixes. Resolve every `CHANGES REQUIRED` finding before moving on. Hooks in [`.claude/settings.json`](.claude/settings.json) prompt for both checkpoints, but the obligation is this rule, not the hook — honour it if hooks are disabled.
|
||||
|
||||
### Naming
|
||||
|
||||
Name variables and functions for **intent**, not mechanism. Naming is the biggest readability lever — avoid generic placeholders (`data`, `handle`, `process`, `tmp`, `helper`, `manager`), type-suffix noise (`nameStr`, `agentList`, `tokenObj`), and mechanism-flavored names (`tickNow`, `hbResp`, `dataObj`).
|
||||
|
||||
Reference in New Issue
Block a user