diff --git a/.claude/skills/git-commit/SKILL.md b/.claude/skills/git-commit/SKILL.md index c236b9e3f..a49bfbff5 100644 --- a/.claude/skills/git-commit/SKILL.md +++ b/.claude/skills/git-commit/SKILL.md @@ -88,17 +88,22 @@ Use the project subsystem as scope. Examples: - Never combine unrelated changes (e.g., don't mix a crash fix with new features). - When in doubt, prefer more smaller commits over fewer large ones. -### Clerk safety valve (`[clerk-skip]`) +### Clerk safety valve (`Clerk-Skip:` trailer) The pre-push clerk reviews each commit for D-record consistency. For a pure bulk content commit — e.g. shipping thousands of generated planetary description files -— that review is moot and just burns agents on noise. Put the token `[clerk-skip]` -anywhere in the commit message and `tooling/clerk-review` auto-approves that commit +— that review is moot and just burns agents on noise. Add a `Clerk-Skip:` **trailer +line** to the commit message and `tooling/clerk-review` auto-approves that commit without spawning an agent. Use it only for content/data dumps, never for commits that touch `decisions/`, code, or ticket-bearing work. +It must be a trailer (a line starting with `Clerk-Skip:`), not inline prose — that +way a commit that merely *mentions* the token in its subject or body isn't skipped. + ``` -data(content): generate 5000 planetary description files [clerk-skip] +data(content): generate 5000 planetary description files + +Clerk-Skip: bulk generated content, no D-record surface ``` ### Examples diff --git a/.config/hooks/pre-push b/.config/hooks/pre-push index 592ea2b9d..2241c909f 100755 --- a/.config/hooks/pre-push +++ b/.config/hooks/pre-push @@ -199,12 +199,16 @@ if [ -x "$REPO_ROOT/tooling/clerk-review" ] && [ "${SR_SKIP_CLERK:-0}" != "1" ]; CLERK_VERDICT=$("$REPO_ROOT/tooling/clerk-review" 2>&1 | tee /dev/stderr | tail -1) if [ "$CLERK_VERDICT" = "APPROVED" ]; then echo "pre-push: clerk — APPROVED" - elif [ "$CLERK_VERDICT" = "TIMEOUT" ]; then - echo "pre-push: clerk — TIMEOUT (findings may arrive later at .cache/pre-push-review.md)" - ERRORS=$((ERRORS + 1)) - else + elif [ "$CLERK_VERDICT" = "REJECTED" ]; then echo "pre-push: clerk — REJECTED (see .cache/pre-push-review.md)" ERRORS=$((ERRORS + 1)) + elif [ "$CLERK_VERDICT" = "INCOMPLETE" ]; then + echo "pre-push: clerk — INCOMPLETE (some reviews didn't finish; NOT blocking)" + echo " See .cache/pre-push-review.md. For a full verdict: raise SR_CLERK_MAX_TURNS / SR_CLERK_TIMEOUT," + echo " or add a 'Clerk-Skip:' trailer to bulk-content commits." + else + echo "pre-push: clerk — '$CLERK_VERDICT' unrecognized; treating as block (see .cache/pre-push-review.md)" + ERRORS=$((ERRORS + 1)) fi else echo "pre-push: clerk review — skipped (not installed or SR_SKIP_CLERK=1)" diff --git a/tooling/clerk-review b/tooling/clerk-review index cad1d72a4..de136eed2 100755 --- a/tooling/clerk-review +++ b/tooling/clerk-review @@ -3,32 +3,41 @@ Clerk pre-push review — checks D-record consistency, ticket drift, and decision contradictions against the diff that would be pushed. -Large pushes are reviewed one commit at a time — commits are the logical units -(each /git-commit is one coherent change), so each clerk agent reviews a -self-contained change *and its commit message* (enabling the "does this commit's -implementation match ticket #NNN?" check). Commits are reviewed by parallel clerk -agents and the verdicts aggregated. This avoids the single-agent failure mode -where one `claude -p` ran out of turns on a huge combined diff and never emitted -a verdict (which defaulted to REJECTED). +Reviewed one commit at a time — commits are the logical units (each /git-commit is +one coherent change), so each clerk agent reviews a self-contained change *and its +commit message*. Commits are reviewed by a bounded pool of parallel clerk agents +and the per-commit verdicts aggregated. -Safety valve: a commit whose message contains the token `[clerk-skip]` is -auto-approved without spawning an agent — for bulk content commits (e.g. shipping -thousands of generated planetary description files) where D-record review is moot -and the diff would only burn agents on noise. +Three outcomes per commit: + APPROVED no contradiction found. + REJECTED a concrete contradiction with a named, active D-record. HARD BLOCK. + INCOMPLETE the review could not finish — timed out, ran out of turns, or emitted + no clear verdict. This is NOT a contradiction; it is non-blocking by + default (the push proceeds with a warning). Raise the budget knobs or + add a `Clerk-Skip:` trailer to avoid it. -Outputs exactly one word as the LAST stdout line: APPROVED, REJECTED, or TIMEOUT. +Why INCOMPLETE exists: an unfinished review must never read as "hard contradiction +found." The previous version defaulted no-verdict / max-turns to REJECTED, which +turned every slow review into a false block (e.g. a CHANGELOG-only commit). + +Safety valve: a commit message with a `Clerk-Skip:` trailer line is auto-approved +without spawning an agent — for bulk content commits (e.g. shipping thousands of +generated planetary description files) where D-record review is moot. A *trailer* +(a line starting with `Clerk-Skip:`) is required, so merely mentioning the token in +prose or a subject line does not trip the valve. + +Outputs exactly one word as the LAST stdout line: APPROVED, REJECTED, or INCOMPLETE. Writes verbose findings to .cache/pre-push-review.md. Exit codes: - 0 = APPROVED - 1 = REJECTED (hard contradiction with an active D-record in >=1 commit) - 2 = TIMEOUT (>=1 commit review timed out, none rejected) + 0 = APPROVED or INCOMPLETE (non-blocking) + 1 = REJECTED (>=1 commit contradicts an active D-record) Env knobs: SR_CLERK_COMMIT_BUDGET per-commit diff char cap (default 120000, truncates) - SR_CLERK_WORKERS parallel clerk agents (default 4) - SR_CLERK_MAX_TURNS turns per clerk agent (default 6) - SR_CLERK_TIMEOUT per-commit timeout seconds (default 150) + SR_CLERK_WORKERS parallel clerk agents (default 4) + SR_CLERK_MAX_TURNS turns per clerk agent (default 15) + SR_CLERK_TIMEOUT per-commit timeout seconds (default 300) Usage: tooling/clerk-review run the review, print verdict @@ -36,6 +45,7 @@ Usage: """ import os +import re import subprocess import sys from concurrent.futures import ThreadPoolExecutor, as_completed @@ -50,11 +60,13 @@ FINDINGS_FILE = CACHE_DIR / "pre-push-review.md" COMMIT_BUDGET = int(os.environ.get("SR_CLERK_COMMIT_BUDGET", "120000")) WORKERS = int(os.environ.get("SR_CLERK_WORKERS", "4")) -MAX_TURNS = os.environ.get("SR_CLERK_MAX_TURNS", "6") -TIMEOUT_SECONDS = int(os.environ.get("SR_CLERK_TIMEOUT", "150")) +MAX_TURNS = os.environ.get("SR_CLERK_MAX_TURNS", "15") +TIMEOUT_SECONDS = int(os.environ.get("SR_CLERK_TIMEOUT", "300")) -# Safety valve: a commit message containing this token is auto-approved (no agent). -SKIP_TOKEN = "[clerk-skip]" +# Safety valve: a commit message with a "Clerk-Skip:" trailer line is auto-approved +# (no agent). A trailer (line-start) is required so that merely mentioning the token +# in prose or a subject line does not trip the valve. +SKIP_TRAILER = re.compile(r"(?im)^[ \t]*clerk-skip[ \t]*:") CLERK_PROMPT = """You are the CLERK, the institutional guardrail for The Settled Reach. @@ -71,16 +83,21 @@ consistency, ticket drift, and decision contradictions. ## Your task -- Does this commit contradict any active D-record? (grep decisions/*.md for relevant keywords) -- If the commit's code/text references a D/Q/R-ID, does that ID exist and is it active? -- If the commit message references ticket #NNN, does the change match the ticket? -Surface any open Q-records relevant to the changed files. +Be efficient — a few targeted greps of decisions/*.md, then conclude. Check: +- Does this commit contradict any active D-record? +- If the commit's code/text cites a D/Q/R-ID, does that ID exist and is it active? +- If the commit message cites ticket #NNN, does the change match the ticket? +Note any relevant open Q-records. -## Output format +## Output format (REQUIRED) -First write your detailed findings. Then, on the VERY LAST LINE, output exactly -one word — APPROVED or REJECTED. REJECTED only for a hard contradiction with an -active D-record; drift, open Q-records, and suggestions are findings, not blocks. +Write brief findings, then on the VERY LAST LINE output exactly one word: +APPROVED or REJECTED. + +- REJECTED *only* for a concrete contradiction with a named, active D-record. +- Drift, stale references, open Q-records, and suggestions are findings, NOT blocks -> APPROVED. +- If you did not find a concrete contradiction, output APPROVED — even if you could + not check everything. "Unsure / didn't finish" means APPROVED, never REJECTED. """ @@ -112,8 +129,8 @@ def list_commits(rng): def commit_unit(sha): """Return (subject, text, skip) for one commit. - `skip` is True when the commit message contains SKIP_TOKEN (the safety valve); - `text` is the message + diff, truncated to COMMIT_BUDGET. + `skip` is True when the commit message has a `Clerk-Skip:` trailer (the safety + valve); `text` is the message + diff, truncated to COMMIT_BUDGET. """ subject = subprocess.check_output( ["git", "show", "-s", "--format=%h %s", sha], text=True @@ -121,7 +138,7 @@ def commit_unit(sha): message = subprocess.check_output( ["git", "show", "-s", "--format=%B", sha], text=True ) - skip = SKIP_TOKEN.lower() in message.lower() + skip = bool(SKIP_TRAILER.search(message)) text = subprocess.check_output( ["git", "show", "--format=fuller", sha], text=True ) @@ -131,7 +148,11 @@ def commit_unit(sha): def run_clerk(label, diff_text, index): - """Spawn one clerk agent over a commit; return (verdict, findings).""" + """Spawn one clerk agent over a commit; return (verdict, findings). + + verdict is APPROVED, REJECTED, or INCOMPLETE. INCOMPLETE covers timeout, empty + output, and no-clear-verdict — none of which is a contradiction. + """ prompt = CLERK_PROMPT.format(label=label, index=index, diff=diff_text) try: result = subprocess.run( @@ -141,17 +162,17 @@ def run_clerk(label, diff_text, index): ) output = result.stdout.strip() except subprocess.TimeoutExpired: - return "TIMEOUT", f"{label}: clerk timed out after {TIMEOUT_SECONDS}s." + return "INCOMPLETE", f"{label}: review timed out after {TIMEOUT_SECONDS}s (not a contradiction)." if not output: - return "REJECTED", f"{label}: clerk produced no output." + return "INCOMPLETE", f"{label}: clerk produced no output (not a contradiction)." last_line = output.splitlines()[-1].strip().upper() if last_line == "APPROVED": return "APPROVED", output if last_line == "REJECTED": return "REJECTED", output - return "REJECTED", output + "\n\n(No clear verdict on last line — defaulting to REJECTED)" + return "INCOMPLETE", output + "\n\n(No clear verdict on last line — recorded as INCOMPLETE, not a contradiction.)" def load_index(): @@ -178,20 +199,20 @@ def main(): print(f"clerk plan: {len(commits)} commit(s) over {rng} " f"(budget {COMMIT_BUDGET} chars, {WORKERS} workers, {MAX_TURNS} turns each)") for i, (subject, text, skip) in enumerate(units): - tag = "SKIP (auto-approve)" if skip else "review" + tag = "SKIP (Clerk-Skip:)" if skip else "review" print(f" commit {i + 1}/{len(commits)}: {len(text):>9} chars [{tag}] {subject}") return 0 index = load_index() reviewable = sum(1 for _, _, skip in units if not skip) print(f" clerk: {len(commits)} commit(s) — {reviewable} to review, " - f"{len(commits) - reviewable} auto-approved ([clerk-skip]); {WORKERS} parallel...", + f"{len(commits) - reviewable} auto-approved (Clerk-Skip:); {WORKERS} parallel...", file=sys.stderr) def task(i, subject, text, skip): label = f"commit {i + 1}/{len(commits)} ({subject})" if skip: - return i, label, "APPROVED", f"{label}: auto-approved via [clerk-skip] safety valve." + return i, label, "APPROVED", f"{label}: auto-approved via Clerk-Skip: trailer." verdict, findings = run_clerk(label, text, index) return i, label, verdict, findings @@ -206,14 +227,17 @@ def main(): verdicts = [r[1] for r in results] if "REJECTED" in verdicts: overall = "REJECTED" - elif "TIMEOUT" in verdicts: - overall = "TIMEOUT" + elif "INCOMPLETE" in verdicts: + overall = "INCOMPLETE" else: overall = "APPROVED" + n_rej = verdicts.count("REJECTED") + n_inc = verdicts.count("INCOMPLETE") parts = [ f"# Clerk Pre-Push Review\n\n**Overall verdict: {overall}**\n", - f"Reviewed {len(commits)} commit(s) over {rng}.\n", + f"Reviewed {len(commits)} commit(s) over {rng} — " + f"{verdicts.count('APPROVED')} approved, {n_rej} rejected, {n_inc} incomplete.\n", ] for label, verdict, findings in results: parts.append(f"\n---\n\n## {label} — {verdict}\n\n{findings}\n") @@ -222,7 +246,7 @@ def main(): print(f" clerk: overall verdict — {overall} (details: {FINDINGS_FILE})", file=sys.stderr) sys.stderr.flush() print(overall) - return {"APPROVED": 0, "REJECTED": 1, "TIMEOUT": 2}[overall] + return 1 if overall == "REJECTED" else 0 if __name__ == "__main__":