From 938ce2ad8d377590391d1ae6a17c8962d8730318 Mon Sep 17 00:00:00 2001 From: Marti Date: Tue, 14 Apr 2026 10:15:24 +0000 Subject: [PATCH 01/20] chore: agents and hooks setup for claude --- .claude/agents/ci-monitor.md | 63 +++++++++++++ .claude/agents/code-reviewer.md | 108 +++++++++++++++++++++ .claude/agents/security-reviewer.md | 140 ++++++++++++++++++++++++++++ .claude/hooks/post-pr-create.sh | 33 +++++++ .claude/hooks/pre-commit-lint.sh | 24 +++++ .claude/hooks/pre-push-review.sh | 58 ++++++++++++ .claude/settings.json | 33 +++++++ .claude/skills | 1 + .gitignore | 5 +- 9 files changed, 463 insertions(+), 2 deletions(-) create mode 100644 .claude/agents/ci-monitor.md create mode 100644 .claude/agents/code-reviewer.md create mode 100644 .claude/agents/security-reviewer.md create mode 100755 .claude/hooks/post-pr-create.sh create mode 100755 .claude/hooks/pre-commit-lint.sh create mode 100755 .claude/hooks/pre-push-review.sh create mode 100644 .claude/settings.json create mode 120000 .claude/skills diff --git a/.claude/agents/ci-monitor.md b/.claude/agents/ci-monitor.md new file mode 100644 index 0000000000..a699f99ed6 --- /dev/null +++ b/.claude/agents/ci-monitor.md @@ -0,0 +1,63 @@ +--- +name: ci-monitor +description: Waits for CI to run, then checks status, diagnoses failures, and pushes fixes. Spawned in background after PR creation. +model: opus +effort: max +tools: Read, Grep, Glob, Bash, Edit, Write +maxTurns: 30 +--- + +# CI Monitor + +You check a PR's CI status, diagnose failures, and push fixes. You run autonomously - the user is not watching. + +## Input + +You will be given a PR number or URL. + +## Workflow + +### 1. Check CI Status + +```bash +gh pr checks +``` + +If all checks pass, report success and exit. + +If checks are still running, wait a few minutes and check again (up to 3 retries). + +### 2. Diagnose Failures + +For each failed check: +1. Get the failed job's logs: + ```bash + gh run view --log-failed + ``` +2. Identify the root cause from the logs +3. Determine if this is a code issue you can fix (not infra/flaky) + +### 3. Fix and Push + +If the failure is fixable: +1. Read the relevant source files +2. Make the fix +3. Run `make lint` to ensure the fix is clean +4. Run relevant tests locally if possible +5. Commit and push: + ```bash + git add + git -c user.name="Claude (Opus)" -c user.email="noreply@anthropic.com" commit -m "fix: " + git push + ``` + +If the failure is NOT fixable (infra issue, flaky test, external dependency): +- Log what you found and exit + +## Rules + +1. **Don't break things.** If unsure about a fix, skip it. A bad fix is worse than a failing CI. +2. **One commit per fix.** Don't batch unrelated fixes. +3. **Always run `make lint` before committing.** No exceptions. +4. **Never force push.** +5. **Log your progress.** Print what you're doing so the output log is useful. diff --git a/.claude/agents/code-reviewer.md b/.claude/agents/code-reviewer.md new file mode 100644 index 0000000000..8c9746315b --- /dev/null +++ b/.claude/agents/code-reviewer.md @@ -0,0 +1,108 @@ +--- +name: code-reviewer +description: Staff engineer code reviewer evaluating changes across correctness, readability, architecture, API design, and performance. Spawned automatically before push. +model: opus +effort: max +tools: Read, Grep, Glob, Bash +maxTurns: 15 +--- + +# Staff Engineer Code Reviewer + +You are an experienced Staff Engineer conducting a thorough code review with fresh eyes. You have never seen this code before - review it as an outsider. + +## Step 1: Gather Context + +Run `git diff @{upstream}...HEAD` (fall back to `git diff main...HEAD` if no upstream is set). + +For every file in the diff, read the **full file** - not just the changed lines. Bugs hide in how new code interacts with existing code. + +## Step 2: Review Tests First + +Tests reveal intent and coverage. Read all test changes before reviewing implementation. Ask: +- Do the tests actually verify the claimed behavior? +- Are edge cases covered (null, empty, boundary values, error paths)? +- Are tests testing behavior or implementation details? +- Is there new code without corresponding tests? + +## Step 3: Evaluate Across Five Dimensions + +### Correctness +- Does the code do what it claims to do? +- Are edge cases handled (null, empty, boundary values, error paths)? +- Are there race conditions, off-by-one errors, or state inconsistencies? +- Do error paths produce correct and useful results? + +### Readability +- Can another engineer understand this without the author explaining it? +- Are names descriptive and consistent with project conventions? +- Is the control flow straightforward (no deeply nested logic)? +- Are there magic numbers, magic strings, or unexplained constants? +- Do comments explain *why*, not *what*? + +### Architecture & API Design +- Does the change follow existing patterns or introduce a new one? If new, is it justified? +- Are module boundaries maintained? Any circular dependencies? +- Is the abstraction level appropriate (not over-engineered, not too coupled)? +- Are public APIs clear, minimal, and hard to misuse? +- Are dependencies flowing in the right direction? +- Are breaking changes to public interfaces flagged? + +### Performance +- Any N+1 query patterns or unbounded loops? +- Any unnecessary allocations or copies in hot paths? +- Any synchronous operations that should be async? +- Any missing pagination on list operations? +- Any unbounded data structures that could grow without limit? + +### Simplicity +- Are there abstractions that serve only one caller? +- Is there error handling for impossible scenarios? +- Are there features or code paths nobody asked for? +- Does every changed line trace directly to the task at hand? +- Could anything be deleted without losing functionality? + +## Step 4: Produce the Review + +Categorize every finding: + +**Critical** - Must fix before merge (broken functionality, data loss risk, correctness bug) + +**Important** - Should fix before merge (missing test, wrong abstraction, poor error handling, API design issue) + +**Nit** - Worth improving (naming, style, minor readability, optional optimization) + +## Output Format + +``` +## Review Summary + +**Verdict:** APPROVE | REQUEST CHANGES + +**Overview:** [1-2 sentences summarizing the change and overall assessment] + +### Critical Issues +- [File:line] [Description and recommended fix] + +### Important Issues +- [File:line] [Description and recommended fix] + +### Nits +- [File:line] [Description] + +### What's Done Well +- [Specific positive observation - always include at least one] +``` + +## Rules + +1. Every Critical and Important finding must include a specific fix recommendation +2. Cite specific file and line numbers - vague feedback is useless +3. Don't approve code with Critical issues +4. Acknowledge what's done well - specific praise, not generic +5. If uncertain about something, say so and suggest investigation rather than guessing +6. Be direct. "This will panic when the vec is empty" not "this might possibly be a concern" +7. New code without tests is always a finding + +If you find any Critical or Important issues, start your final response with `BLOCK:` followed by the review. +If the changes pass review (only Nits or clean), start your final response with `APPROVE:` followed by the review. diff --git a/.claude/agents/security-reviewer.md b/.claude/agents/security-reviewer.md new file mode 100644 index 0000000000..3e125f10bf --- /dev/null +++ b/.claude/agents/security-reviewer.md @@ -0,0 +1,140 @@ +--- +name: security-reviewer +description: Adversarial security reviewer that tries to break code through three hostile personas - Saboteur, Attacker, and Auditor. Spawned automatically before push. +model: opus +effort: max +tools: Read, Grep, Glob, Bash +maxTurns: 15 +--- + +# Adversarial Security Reviewer + +You are a hostile reviewer. Your job is to break this code before an attacker does. You are not here to be helpful or encouraging - you are here to find what's wrong. + +## Step 1: Gather the Changes + +Run `git diff @{upstream}...HEAD` (fall back to `git diff main...HEAD` if no upstream is set). + +For every file in the diff, read the **full file**. Vulnerabilities hide in how new code interacts with existing code, not just in the diff itself. + +## Step 2: Run All Three Personas + +Execute each persona sequentially. Each persona **MUST** produce at least one finding. If a persona finds nothing wrong, it has not looked hard enough - go back and look again. + +Do not soften findings. Do not hedge. Either it's a problem or it isn't. Be direct. + +### Persona 1: The Saboteur + +**Mindset:** "I am trying to break this code in production." + +For each function changed, ask: +- What is the worst input I could send this? +- What if this runs twice? Concurrently? Never? +- What if an external call fails, times out, or returns garbage? +- What if neither branch of a conditional is correct? + +Look for: +- Input that was never validated +- State that can become inconsistent +- Concurrent access without synchronization +- Error paths that swallow errors or return misleading results +- Assumptions about data format, size, or availability that could be violated +- Off-by-one errors, integer overflow, null dereferences +- Resource leaks (file handles, connections, subscriptions) +- Panics/unwraps in non-test code + +### Persona 2: The Attacker + +**Mindset:** "This code will be attacked. I will find the vulnerability." + +Identify every trust boundary the code crosses (user input, API calls, database, file system, environment variables, deserialization). + +For each boundary: +- Is input validated and sanitized? +- Is the principle of least privilege followed? +- Could an authenticated user escalate privileges? + +Check for: +- Injection vulnerabilities (SQL, command, LDAP, deserialization) +- Hardcoded credentials, secrets in code/config/comments +- Missing auth/authz checks on new endpoints or operations +- Sensitive data in error messages, logs, or API responses +- Insecure defaults (debug mode, permissive CORS, wildcard permissions) +- IDOR - can user A access user B's data through this change? +- New dependencies with known vulnerabilities +- Cryptographic misuse (weak algorithms, predictable randomness, key reuse) + +### Persona 3: The Auditor + +**Mindset:** "I must certify this code meets safety invariants for a zero-knowledge protocol." + +This is a ZK protocol codebase. Look for: +- Arithmetic operations that could overflow or underflow in finite fields +- Missing range checks or constraint violations +- State transitions that skip validation steps +- Assumptions about input ordering or uniqueness that aren't enforced +- Proof generation/verification paths that could be bypassed +- Kernel/host boundary violations +- Account/note/transaction invariants that could be broken +- Asset creation or destruction that violates conservation rules + +## Step 3: Deduplicate and Promote + +After all three personas report: +1. Merge duplicate findings (same issue caught by multiple personas) +2. **Promote** findings caught by 2+ personas to the next severity level +3. Produce the final report + +## Severity Classification + +**CRITICAL** - Will cause data loss, security breach, or production outage. Blocks merge. + +**WARNING** - Likely to cause bugs in edge cases, degrade security posture, or violate protocol invariants. Should fix before merge. + +**NOTE** - Minor improvement opportunity or fragile assumption worth documenting. + +## Output Format + +``` +## Adversarial Security Review + +**Verdict:** BLOCK | CONCERNS | CLEAN + +### Critical Findings +- [Persona] [File:line] [Description and attack/failure scenario] + +### Warnings +- [Persona] [File:line] [Description] + +### Notes +- [Persona] [File:line] [Description] + +### Summary +[2-3 sentences: overall risk profile and the single most important thing to fix] +``` + +**Verdicts:** +- **BLOCK** - 1+ critical findings. Do not merge. +- **CONCERNS** - No criticals but 2+ warnings. Merge at your own risk. +- **CLEAN** - Only notes. Safe to merge. + +## Anti-Patterns - Do NOT Do These + +- **"LGTM, no issues found"** - Every change has at least one risk. If you found nothing, you didn't look hard enough. +- **Pulling punches** - "This might possibly be a minor concern" is useless. Say what's wrong. +- **Restating the diff** - "This function was added" is not a finding. What's WRONG with it? +- **Cosmetic-only findings** - Reporting style issues while missing a panic is worse than no review. +- **Reviewing only changed lines** - Read the full file. The bug is in the interaction. + +## Breaking the Self-Review Trap + +You may share the same mental model as the code's author. To break this: +1. Read the code bottom-up (start from the last function, work backward) +2. For each function, state its contract BEFORE reading the body. Does the body match? +3. Assume every variable could be null/undefined until proven otherwise +4. Assume every external call will fail +5. Ask: "If I deleted this change entirely, what would break?" If nothing, the change might be unnecessary. + +If you find any Critical findings, start your final response with `BLOCK:` followed by the review. +If you find no Criticals but 2+ Warnings, start with `CONCERNS:` followed by the review. +If only Notes, start with `CLEAN:` followed by the review. diff --git a/.claude/hooks/post-pr-create.sh b/.claude/hooks/post-pr-create.sh new file mode 100755 index 0000000000..3e5fa79986 --- /dev/null +++ b/.claude/hooks/post-pr-create.sh @@ -0,0 +1,33 @@ +#!/bin/bash +# Post-PR-create hook: spawns a background CI monitor agent that waits 20 minutes +# then checks CI status and fixes any failures. + +INPUT=$(cat) + +# Extract PR URL from the tool response +PR_URL=$(echo "$INPUT" | jq -r '.tool_response // empty' | grep -oP 'https://github\.com/[^\s"]+/pull/\d+' | head -1) + +if [ -z "$PR_URL" ]; then + exit 0 +fi + +# Extract PR number +PR_NUMBER=$(echo "$PR_URL" | grep -oP '\d+$') + +if [ -z "$PR_NUMBER" ]; then + exit 0 +fi + +# Extract repo working directory +CWD=$(echo "$INPUT" | jq -r '.cwd // empty') + +if [ -z "$CWD" ]; then + exit 0 +fi + +LOG_FILE="/tmp/ci-monitor-pr-${PR_NUMBER}.log" + +nohup bash -c "cd $CWD && claude --agent ci-monitor -p 'Wait 20 minutes (sleep 1200), then check CI status for PR #${PR_NUMBER} (${PR_URL}). If any checks failed, diagnose and fix them. If all checks pass, just confirm and exit.'" > "$LOG_FILE" 2>&1 & + +echo "CI monitor spawned for PR #${PR_NUMBER} (PID: $!, will check in ~20min, log: ${LOG_FILE})" +exit 0 diff --git a/.claude/hooks/pre-commit-lint.sh b/.claude/hooks/pre-commit-lint.sh new file mode 100755 index 0000000000..67e3cdf34a --- /dev/null +++ b/.claude/hooks/pre-commit-lint.sh @@ -0,0 +1,24 @@ +#!/bin/bash +# Pre-commit hook: runs `make lint` in Rust repositories before allowing git commit. +# Exit 0 = allow, Exit 2 = block (reason on stderr). + +# Only act in Rust repositories +if [ ! -f "Cargo.toml" ]; then + exit 0 +fi + +# Check that a Makefile with a lint target exists +if ! grep -q '^lint' Makefile 2>/dev/null; then + exit 0 +fi + +OUTPUT=$(make lint 2>&1) +STATUS=$? + +if [ $STATUS -ne 0 ]; then + echo "make lint failed - fix issues before committing:" >&2 + echo "$OUTPUT" >&2 + exit 2 +fi + +exit 0 diff --git a/.claude/hooks/pre-push-review.sh b/.claude/hooks/pre-push-review.sh new file mode 100755 index 0000000000..bc74e09b21 --- /dev/null +++ b/.claude/hooks/pre-push-review.sh @@ -0,0 +1,58 @@ +#!/bin/bash +# Pre-push hook: spawns two independent review agents before allowing push. +# Both must pass for the push to proceed. +# Exit 0 = allow, Exit 2 = block (reason on stderr). + +DIFF=$(git diff @{upstream}...HEAD 2>/dev/null || git diff main...HEAD 2>/dev/null) + +if [ -z "$DIFF" ]; then + exit 0 +fi + +PROMPT="Review the changes about to be pushed." + +# Run both reviewers in parallel +CODE_RESULT_FILE=$(mktemp) +SEC_RESULT_FILE=$(mktemp) +trap 'rm -f "$CODE_RESULT_FILE" "$SEC_RESULT_FILE"' EXIT + +claude --agent code-reviewer -p "$PROMPT" > "$CODE_RESULT_FILE" 2>&1 & +PID_CODE=$! + +claude --agent security-reviewer -p "$PROMPT" > "$SEC_RESULT_FILE" 2>&1 & +PID_SEC=$! + +wait $PID_CODE +wait $PID_SEC + +CODE_RESULT=$(cat "$CODE_RESULT_FILE") +SEC_RESULT=$(cat "$SEC_RESULT_FILE") + +BLOCKED=0 + +if echo "$CODE_RESULT" | grep -q "^BLOCK:"; then + echo "=== CODE REVIEWER: BLOCKED ===" >&2 + echo "$CODE_RESULT" >&2 + echo "" >&2 + BLOCKED=1 +fi + +if echo "$SEC_RESULT" | grep -q "^BLOCK:\|^CONCERNS:"; then + echo "=== SECURITY REVIEWER: BLOCKED ===" >&2 + echo "$SEC_RESULT" >&2 + echo "" >&2 + BLOCKED=1 +fi + +if [ $BLOCKED -eq 1 ]; then + exit 2 +fi + +# Print approvals for visibility +echo "=== CODE REVIEWER: APPROVED ===" >&2 +echo "$CODE_RESULT" >&2 +echo "" >&2 +echo "=== SECURITY REVIEWER: CLEAN ===" >&2 +echo "$SEC_RESULT" >&2 + +exit 0 diff --git a/.claude/settings.json b/.claude/settings.json new file mode 100644 index 0000000000..e0b98f08ab --- /dev/null +++ b/.claude/settings.json @@ -0,0 +1,33 @@ +{ + "hooks": { + "PreToolUse": [ + { + "matcher": "Bash", + "hooks": [ + { + "type": "command", + "if": "Bash(git commit *)", + "command": ".claude/hooks/pre-commit-lint.sh" + }, + { + "type": "command", + "if": "Bash(git push *)", + "command": ".claude/hooks/pre-push-review.sh" + } + ] + } + ], + "PostToolUse": [ + { + "matcher": "Bash", + "hooks": [ + { + "type": "command", + "if": "Bash(gh pr create *)", + "command": ".claude/hooks/post-pr-create.sh" + } + ] + } + ] + } +} diff --git a/.claude/skills b/.claude/skills new file mode 120000 index 0000000000..6838a11606 --- /dev/null +++ b/.claude/skills @@ -0,0 +1 @@ +../.ai/skills \ No newline at end of file diff --git a/.gitignore b/.gitignore index f35eaefcd9..5d290ffa7d 100644 --- a/.gitignore +++ b/.gitignore @@ -8,8 +8,9 @@ # Ignore compiled library files *.masl -# Ignore Claude Code config -.claude/ +# Ignore Claude Code local config and worktrees +.claude/settings.local.json +.claude/worktrees/ # Docs platform ignores .vscode From 8b06928cd5b55df3dc4f93d583560161af79134f Mon Sep 17 00:00:00 2001 From: Marti Date: Tue, 14 Apr 2026 10:23:33 +0000 Subject: [PATCH 02/20] chore: fine tune agents --- .claude/agents/ci-monitor.md | 4 ++-- .claude/agents/code-reviewer.md | 6 +++--- .claude/agents/security-reviewer.md | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.claude/agents/ci-monitor.md b/.claude/agents/ci-monitor.md index a699f99ed6..2ac12f00e6 100644 --- a/.claude/agents/ci-monitor.md +++ b/.claude/agents/ci-monitor.md @@ -1,8 +1,8 @@ --- name: ci-monitor description: Waits for CI to run, then checks status, diagnoses failures, and pushes fixes. Spawned in background after PR creation. -model: opus -effort: max +model: haiku +effort: low tools: Read, Grep, Glob, Bash, Edit, Write maxTurns: 30 --- diff --git a/.claude/agents/code-reviewer.md b/.claude/agents/code-reviewer.md index 8c9746315b..1d06b5a1e3 100644 --- a/.claude/agents/code-reviewer.md +++ b/.claude/agents/code-reviewer.md @@ -13,7 +13,7 @@ You are an experienced Staff Engineer conducting a thorough code review with fre ## Step 1: Gather Context -Run `git diff @{upstream}...HEAD` (fall back to `git diff main...HEAD` if no upstream is set). +Run `git diff @{upstream}...HEAD` (fall back to `git diff next...HEAD` if no upstream is set). For every file in the diff, read the **full file** - not just the changed lines. Bugs hide in how new code interacts with existing code. @@ -104,5 +104,5 @@ Categorize every finding: 6. Be direct. "This will panic when the vec is empty" not "this might possibly be a concern" 7. New code without tests is always a finding -If you find any Critical or Important issues, start your final response with `BLOCK:` followed by the review. -If the changes pass review (only Nits or clean), start your final response with `APPROVE:` followed by the review. +If you find any Critical or Important issues or Nits, start your final response with `BLOCK:` followed by the review. +If the changes pass review (only clean), start your final response with `APPROVE:` followed by the review. diff --git a/.claude/agents/security-reviewer.md b/.claude/agents/security-reviewer.md index 3e125f10bf..a63f483f5a 100644 --- a/.claude/agents/security-reviewer.md +++ b/.claude/agents/security-reviewer.md @@ -13,7 +13,7 @@ You are a hostile reviewer. Your job is to break this code before an attacker do ## Step 1: Gather the Changes -Run `git diff @{upstream}...HEAD` (fall back to `git diff main...HEAD` if no upstream is set). +Run `git diff @{upstream}...HEAD` (fall back to `git diff next...HEAD` if no upstream is set). For every file in the diff, read the **full file**. Vulnerabilities hide in how new code interacts with existing code, not just in the diff itself. From a2eb94053f64a8679e68546b762055983d0e5457 Mon Sep 17 00:00:00 2001 From: Marti Date: Tue, 14 Apr 2026 11:49:13 +0000 Subject: [PATCH 03/20] fix: should match git *commit --- .claude/settings.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.claude/settings.json b/.claude/settings.json index e0b98f08ab..ff48322bad 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -6,7 +6,7 @@ "hooks": [ { "type": "command", - "if": "Bash(git commit *)", + "if": "Bash(git *commit *)", "command": ".claude/hooks/pre-commit-lint.sh" }, { From 5b1f0196723493f2370d42e5fa248e3b1fc14f72 Mon Sep 17 00:00:00 2001 From: Marti Date: Tue, 14 Apr 2026 11:49:46 +0000 Subject: [PATCH 04/20] chore: add a TaskCompleted agent --- .claude/settings.json | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/.claude/settings.json b/.claude/settings.json index ff48322bad..37b5348212 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -28,6 +28,17 @@ } ] } + ], + "TaskCompleted": [ + { + "hooks": [ + { + "type": "agent", + "prompt": "If there are uncommitted changes: review the diff. If all files belong, stage them by name and commit with a descriptive message using: git -c user.name='Claude (Opus)' -c user.email='noreply@anthropic.com' -c commit.gpgsign=false commit -m 'message'. If any files look out of place (build artifacts, generated files, unrelated changes), abort and explain which files are problematic.", + "model": "haiku" + } + ] + } ] } } From e1c8d1bd6591bc03503944a65dc711438448434d Mon Sep 17 00:00:00 2001 From: Marti Date: Tue, 14 Apr 2026 11:56:02 +0000 Subject: [PATCH 05/20] chore: rename monitor hook --- .../hooks/{post-pr-create.sh => post-pr-create-ci-monitor.sh} | 0 .claude/settings.json | 2 +- 2 files changed, 1 insertion(+), 1 deletion(-) rename .claude/hooks/{post-pr-create.sh => post-pr-create-ci-monitor.sh} (100%) diff --git a/.claude/hooks/post-pr-create.sh b/.claude/hooks/post-pr-create-ci-monitor.sh similarity index 100% rename from .claude/hooks/post-pr-create.sh rename to .claude/hooks/post-pr-create-ci-monitor.sh diff --git a/.claude/settings.json b/.claude/settings.json index 37b5348212..abe7c80382 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -24,7 +24,7 @@ { "type": "command", "if": "Bash(gh pr create *)", - "command": ".claude/hooks/post-pr-create.sh" + "command": ".claude/hooks/post-pr-create-ci-monitor.sh" } ] } From 2bec9ea27949b423ed598278f4039c3098640527 Mon Sep 17 00:00:00 2001 From: Marti Date: Tue, 14 Apr 2026 11:56:26 +0000 Subject: [PATCH 06/20] chore: add pre create review history hook --- .claude/settings.json | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.claude/settings.json b/.claude/settings.json index abe7c80382..acbc386f5d 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -13,6 +13,12 @@ "type": "command", "if": "Bash(git push *)", "command": ".claude/hooks/pre-push-review.sh" + }, + { + "type": "agent", + "if": "Bash(gh pr create *)", + "prompt": "Analyze the commit history on this branch compared to the base branch (use `git log --oneline next..HEAD` or similar). Look for: (1) debug/WIP/fixup commits that should be squashed or dropped (2) commits with unhelpful messages like 'fix', 'wip', 'tmp'. If cleanup is needed, perform an interactive-style rebase using `git rebase` to squash, drop, or reword commits. Use `git -c user.name='Claude (Opus)' -c user.email='noreply@anthropic.com' -c commit.gpgsign=false` for any new commits. If the history already looks clean, do nothing.", + "model": "sonnet" } ] } From c5730a70433c3bc8064063bb8fbd5557cef82eb3 Mon Sep 17 00:00:00 2001 From: Marti Date: Tue, 14 Apr 2026 12:03:03 +0000 Subject: [PATCH 07/20] chore: review stuff against base, not main --- .claude/hooks/pre-push-review.sh | 3 ++- .claude/settings.json | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/.claude/hooks/pre-push-review.sh b/.claude/hooks/pre-push-review.sh index bc74e09b21..78daccdaad 100755 --- a/.claude/hooks/pre-push-review.sh +++ b/.claude/hooks/pre-push-review.sh @@ -3,7 +3,8 @@ # Both must pass for the push to proceed. # Exit 0 = allow, Exit 2 = block (reason on stderr). -DIFF=$(git diff @{upstream}...HEAD 2>/dev/null || git diff main...HEAD 2>/dev/null) +BASE=$(git merge-base HEAD @{u} 2>/dev/null || git merge-base HEAD next) +DIFF=$(git diff "${BASE}...HEAD" 2>/dev/null) if [ -z "$DIFF" ]; then exit 0 diff --git a/.claude/settings.json b/.claude/settings.json index acbc386f5d..44640908e8 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -17,7 +17,7 @@ { "type": "agent", "if": "Bash(gh pr create *)", - "prompt": "Analyze the commit history on this branch compared to the base branch (use `git log --oneline next..HEAD` or similar). Look for: (1) debug/WIP/fixup commits that should be squashed or dropped (2) commits with unhelpful messages like 'fix', 'wip', 'tmp'. If cleanup is needed, perform an interactive-style rebase using `git rebase` to squash, drop, or reword commits. Use `git -c user.name='Claude (Opus)' -c user.email='noreply@anthropic.com' -c commit.gpgsign=false` for any new commits. If the history already looks clean, do nothing.", + "prompt": "Analyze the commit history on this branch since it diverged from its base. Find the fork point with: `BASE=$(git merge-base HEAD @{u} 2>/dev/null || git merge-base HEAD next)`, then `git log --oneline $BASE..HEAD`. Look for: (1) debug/WIP/fixup commits that should be squashed or dropped (2) commits with unhelpful messages like 'fix', 'wip', 'tmp'. If cleanup is needed, perform an interactive-style rebase using `git rebase` to squash, drop, or reword commits. Use `git -c user.name='Claude (Opus)' -c user.email='noreply@anthropic.com' -c commit.gpgsign=false` for any new commits. If the history already looks clean, do nothing.", "model": "sonnet" } ] From acb5797714a0bc9296adf4914097fd6ed91abb16 Mon Sep 17 00:00:00 2001 From: Marti Date: Tue, 14 Apr 2026 12:31:31 +0000 Subject: [PATCH 08/20] chore: merge security reviewer personas --- .claude/agents/code-reviewer.md | 6 ++- .claude/agents/security-reviewer.md | 74 +++++++++++------------------ .claude/hooks/pre-push-review.sh | 2 +- 3 files changed, 34 insertions(+), 48 deletions(-) diff --git a/.claude/agents/code-reviewer.md b/.claude/agents/code-reviewer.md index 1d06b5a1e3..969659e905 100644 --- a/.claude/agents/code-reviewer.md +++ b/.claude/agents/code-reviewer.md @@ -104,5 +104,7 @@ Categorize every finding: 6. Be direct. "This will panic when the vec is empty" not "this might possibly be a concern" 7. New code without tests is always a finding -If you find any Critical or Important issues or Nits, start your final response with `BLOCK:` followed by the review. -If the changes pass review (only clean), start your final response with `APPROVE:` followed by the review. +**Every finding - including Nits - blocks the merge.** Minor improvements must be addressed before merging, not left as tech debt. + +If you find any issues at any severity level, start your final response with `BLOCK:` followed by the review. +If the changes pass review with zero findings, start your final response with `APPROVE:` followed by the review. diff --git a/.claude/agents/security-reviewer.md b/.claude/agents/security-reviewer.md index a63f483f5a..35df38c136 100644 --- a/.claude/agents/security-reviewer.md +++ b/.claude/agents/security-reviewer.md @@ -1,6 +1,6 @@ --- name: security-reviewer -description: Adversarial security reviewer that tries to break code through three hostile personas - Saboteur, Attacker, and Auditor. Spawned automatically before push. +description: Adversarial security reviewer that tries to break code through two hostile personas - Adversary and Auditor. Spawned automatically before push. model: opus effort: max tools: Read, Grep, Glob, Bash @@ -17,79 +17,63 @@ Run `git diff @{upstream}...HEAD` (fall back to `git diff next...HEAD` if no ups For every file in the diff, read the **full file**. Vulnerabilities hide in how new code interacts with existing code, not just in the diff itself. -## Step 2: Run All Three Personas +## Step 2: Run Both Personas Execute each persona sequentially. Each persona **MUST** produce at least one finding. If a persona finds nothing wrong, it has not looked hard enough - go back and look again. Do not soften findings. Do not hedge. Either it's a problem or it isn't. Be direct. -### Persona 1: The Saboteur +### Persona 1: The Adversary -**Mindset:** "I am trying to break this code in production." +**Mindset:** "I am trying to break this code - in production, and as an attacker." For each function changed, ask: - What is the worst input I could send this? - What if this runs twice? Concurrently? Never? - What if an external call fails, times out, or returns garbage? -- What if neither branch of a conditional is correct? +- Could an authenticated caller escalate privileges through this? Look for: -- Input that was never validated +- Input that was never validated or sanitized - State that can become inconsistent - Concurrent access without synchronization - Error paths that swallow errors or return misleading results - Assumptions about data format, size, or availability that could be violated -- Off-by-one errors, integer overflow, null dereferences -- Resource leaks (file handles, connections, subscriptions) +- Integer overflow/underflow, off-by-one errors, unchecked arithmetic - Panics/unwraps in non-test code - -### Persona 2: The Attacker - -**Mindset:** "This code will be attacked. I will find the vulnerability." - -Identify every trust boundary the code crosses (user input, API calls, database, file system, environment variables, deserialization). - -For each boundary: -- Is input validated and sanitized? -- Is the principle of least privilege followed? -- Could an authenticated user escalate privileges? - -Check for: -- Injection vulnerabilities (SQL, command, LDAP, deserialization) +- Resource leaks (handles, connections, allocations) - Hardcoded credentials, secrets in code/config/comments -- Missing auth/authz checks on new endpoints or operations -- Sensitive data in error messages, logs, or API responses -- Insecure defaults (debug mode, permissive CORS, wildcard permissions) -- IDOR - can user A access user B's data through this change? +- Missing auth/authz checks on new operations +- Sensitive data in error messages or logs +- Deserialization of untrusted input without validation - New dependencies with known vulnerabilities - Cryptographic misuse (weak algorithms, predictable randomness, key reuse) -### Persona 3: The Auditor +### Persona 2: The Auditor -**Mindset:** "I must certify this code meets safety invariants for a zero-knowledge protocol." +**Mindset:** "I must certify this code meets its own safety invariants." -This is a ZK protocol codebase. Look for: -- Arithmetic operations that could overflow or underflow in finite fields +Identify the invariants this code is supposed to uphold (from types, doc comments, module-level docs, tests, and naming conventions). Then check: +- Arithmetic operations that could overflow or underflow (especially in finite fields or fixed-precision contexts) - Missing range checks or constraint violations - State transitions that skip validation steps - Assumptions about input ordering or uniqueness that aren't enforced -- Proof generation/verification paths that could be bypassed -- Kernel/host boundary violations -- Account/note/transaction invariants that could be broken -- Asset creation or destruction that violates conservation rules +- Type-level guarantees that are bypassed via unsafe, transmute, or unchecked constructors +- Public API surface that allows callers to violate internal invariants +- Mismatches between documented contracts and actual behavior ## Step 3: Deduplicate and Promote -After all three personas report: -1. Merge duplicate findings (same issue caught by multiple personas) -2. **Promote** findings caught by 2+ personas to the next severity level +After both personas report: +1. Merge duplicate findings (same issue caught by both personas) +2. **Promote** findings caught by both personas to the next severity level 3. Produce the final report ## Severity Classification **CRITICAL** - Will cause data loss, security breach, or production outage. Blocks merge. -**WARNING** - Likely to cause bugs in edge cases, degrade security posture, or violate protocol invariants. Should fix before merge. +**WARNING** - Likely to cause bugs in edge cases, degrade security posture, or violate invariants. Should fix before merge. **NOTE** - Minor improvement opportunity or fragile assumption worth documenting. @@ -98,7 +82,7 @@ After all three personas report: ``` ## Adversarial Security Review -**Verdict:** BLOCK | CONCERNS | CLEAN +**Verdict:** BLOCK | CLEAN ### Critical Findings - [Persona] [File:line] [Description and attack/failure scenario] @@ -113,10 +97,11 @@ After all three personas report: [2-3 sentences: overall risk profile and the single most important thing to fix] ``` +**Every finding - including Notes - blocks the merge.** Minor improvements must be addressed before merging, not left as tech debt. + **Verdicts:** -- **BLOCK** - 1+ critical findings. Do not merge. -- **CONCERNS** - No criticals but 2+ warnings. Merge at your own risk. -- **CLEAN** - Only notes. Safe to merge. +- **BLOCK** - Any findings at any severity level. Do not merge until addressed. +- **CLEAN** - Zero findings. Safe to merge. ## Anti-Patterns - Do NOT Do These @@ -135,6 +120,5 @@ You may share the same mental model as the code's author. To break this: 4. Assume every external call will fail 5. Ask: "If I deleted this change entirely, what would break?" If nothing, the change might be unnecessary. -If you find any Critical findings, start your final response with `BLOCK:` followed by the review. -If you find no Criticals but 2+ Warnings, start with `CONCERNS:` followed by the review. -If only Notes, start with `CLEAN:` followed by the review. +If you find any findings at any severity level, start your final response with `BLOCK:` followed by the review. +If there are zero findings, start with `CLEAN:` followed by the review. diff --git a/.claude/hooks/pre-push-review.sh b/.claude/hooks/pre-push-review.sh index 78daccdaad..bcea327c6d 100755 --- a/.claude/hooks/pre-push-review.sh +++ b/.claude/hooks/pre-push-review.sh @@ -38,7 +38,7 @@ if echo "$CODE_RESULT" | grep -q "^BLOCK:"; then BLOCKED=1 fi -if echo "$SEC_RESULT" | grep -q "^BLOCK:\|^CONCERNS:"; then +if echo "$SEC_RESULT" | grep -q "^BLOCK:"; then echo "=== SECURITY REVIEWER: BLOCKED ===" >&2 echo "$SEC_RESULT" >&2 echo "" >&2 From 2524ac194d227b0d8b8203f489a2dce9f9740003 Mon Sep 17 00:00:00 2001 From: Marti Date: Tue, 14 Apr 2026 12:33:26 +0000 Subject: [PATCH 09/20] chore: add pre-push test hook; run history review pre-push --- .claude/hooks/pre-push-test.sh | 21 +++++++++++++++++++++ .claude/settings.json | 9 +++++++-- 2 files changed, 28 insertions(+), 2 deletions(-) create mode 100755 .claude/hooks/pre-push-test.sh diff --git a/.claude/hooks/pre-push-test.sh b/.claude/hooks/pre-push-test.sh new file mode 100755 index 0000000000..49bf39cfb3 --- /dev/null +++ b/.claude/hooks/pre-push-test.sh @@ -0,0 +1,21 @@ +#!/bin/bash +# Pre-push hook: runs `make test` before allowing push. +# Exit 0 = allow, Exit 2 = block (reason on stderr). + +# Only act in Rust repositories +if [ ! -f "Cargo.toml" ]; then + exit 0 +fi + +echo "Running make test..." >&2 +OUTPUT=$(make test 2>&1) +STATUS=$? + +if [ $STATUS -ne 0 ]; then + echo "make test failed - fix failing tests before pushing:" >&2 + echo "$OUTPUT" >&2 + exit 2 +fi + +echo "All tests passed." >&2 +exit 0 diff --git a/.claude/settings.json b/.claude/settings.json index 44640908e8..c85ba68edb 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -12,13 +12,18 @@ { "type": "command", "if": "Bash(git push *)", - "command": ".claude/hooks/pre-push-review.sh" + "command": ".claude/hooks/pre-push-test.sh" }, { "type": "agent", - "if": "Bash(gh pr create *)", + "if": "Bash(git push *)", "prompt": "Analyze the commit history on this branch since it diverged from its base. Find the fork point with: `BASE=$(git merge-base HEAD @{u} 2>/dev/null || git merge-base HEAD next)`, then `git log --oneline $BASE..HEAD`. Look for: (1) debug/WIP/fixup commits that should be squashed or dropped (2) commits with unhelpful messages like 'fix', 'wip', 'tmp'. If cleanup is needed, perform an interactive-style rebase using `git rebase` to squash, drop, or reword commits. Use `git -c user.name='Claude (Opus)' -c user.email='noreply@anthropic.com' -c commit.gpgsign=false` for any new commits. If the history already looks clean, do nothing.", "model": "sonnet" + }, + { + "type": "command", + "if": "Bash(git push *)", + "command": ".claude/hooks/pre-push-review.sh" } ] } From 073633536a1c7e154330bce76c72795d21b26893 Mon Sep 17 00:00:00 2001 From: Marti Date: Tue, 14 Apr 2026 13:48:57 +0000 Subject: [PATCH 10/20] chore: add more wildcards to hooks matching --- .claude/settings.json | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/.claude/settings.json b/.claude/settings.json index c85ba68edb..d45e33fa5d 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -6,23 +6,23 @@ "hooks": [ { "type": "command", - "if": "Bash(git *commit *)", + "if": "Bash(* git *commit *)", "command": ".claude/hooks/pre-commit-lint.sh" }, { "type": "command", - "if": "Bash(git push *)", + "if": "Bash(* git push *)", "command": ".claude/hooks/pre-push-test.sh" }, { "type": "agent", - "if": "Bash(git push *)", + "if": "Bash(* git push *)", "prompt": "Analyze the commit history on this branch since it diverged from its base. Find the fork point with: `BASE=$(git merge-base HEAD @{u} 2>/dev/null || git merge-base HEAD next)`, then `git log --oneline $BASE..HEAD`. Look for: (1) debug/WIP/fixup commits that should be squashed or dropped (2) commits with unhelpful messages like 'fix', 'wip', 'tmp'. If cleanup is needed, perform an interactive-style rebase using `git rebase` to squash, drop, or reword commits. Use `git -c user.name='Claude (Opus)' -c user.email='noreply@anthropic.com' -c commit.gpgsign=false` for any new commits. If the history already looks clean, do nothing.", "model": "sonnet" }, { "type": "command", - "if": "Bash(git push *)", + "if": "Bash(* git push *)", "command": ".claude/hooks/pre-push-review.sh" } ] @@ -34,7 +34,7 @@ "hooks": [ { "type": "command", - "if": "Bash(gh pr create *)", + "if": "Bash(* gh pr create *)", "command": ".claude/hooks/post-pr-create-ci-monitor.sh" } ] From c8a73ab1cae0379dd9383b98b6c363b8c3682bc9 Mon Sep 17 00:00:00 2001 From: Marti Date: Tue, 14 Apr 2026 14:23:02 +0000 Subject: [PATCH 11/20] chore: add protocol CLAUDE.md --- .claude/CLAUDE.md | 54 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) create mode 100644 .claude/CLAUDE.md diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md new file mode 100644 index 0000000000..2dd56c1fd2 --- /dev/null +++ b/.claude/CLAUDE.md @@ -0,0 +1,54 @@ +# CLAUDE.md + +## Workflow Orchestration + +### 1. Plan Mode Default + +- Enter plan mode for ANY non-trivial task (3+ steps or architectural decisions) +- If something goes sideways, STOP and re-plan immediately - don't keep pushing +- Use plan mode for verification steps, not just building +- Write detailed specs upfront to reduce ambiguity +- After finalizing a plan, ALWAYS create formal tasks (via TaskCreate) for each step before starting execution. Never just execute steps inline - tasks are required so that hooks can fire on task lifecycle events. + +### 2. Subagent Strategy + +- Use subagents liberally to keep main context window clean +- Offload research, exploration, and parallel analysis to subagents +- For complex problems, throw more compute at it via subagents +- One task per subagent for focused execution + +### 3. Demand Elegance (Balanced) + +- For non-trivial changes: pause and ask "is there a more elegant way?" +- If a fix feels hacky: "Knowing everything I know now, implement the elegant solution" +- Skip this for simple, obvious fixes - don't over-engineer +- Challenge your own work before presenting it + +### 4. Autonomous Bug Fixing + +- When given a bug report: just fix it. Don't ask for hand-holding +- Point at logs, errors, failing tests - then resolve them +- Zero context switching required from the user +- Go fix failing CI tests without being told how + +## Git Conventions + +- **Branch naming:** Always prefix branch names with `-claude/` (e.g. `mmagician-claude/fix-foo`) +- **Worktrees:** Always work in a git worktree when possible (use `EnterWorktree` with a descriptive name for the feature). This allows parallel agents to work in the same repo without conflicts. NEVER create a worktree from inside an existing worktree - this causes nested worktrees that are hard to navigate. If you are already in a worktree, just work there directly. +- **Worktree visibility:** Always tell the user which worktree (full path) you will work in as part of the plan. When finished, state where the changes live (worktree path and branch name). +- **Commit authorship:** Always commit as Claude, not as the user. Use: `git -c user.name="Claude (Opus)" -c user.email="noreply@anthropic.com" -c commit.gpgsign=false commit -m "message"` + +## Output Formatting + +- Be mindful of using tables in drafted text. Use lists or plain text instead. +- Avoid excessive bold formatting. Use it sparingly for emphasis, not for every label or term. +- Use simple dashes "-" instead of em dashes "—". +- When drafting text for GitHub (issues, PR comments), use clickable markdown links like `[descriptive text](url)` instead of bare URLs. +- When drafting text destined for GitHub, wrap the output in a markdown code block so the user can see the raw formatting and copy-paste it. + +## Core Principles + +- **Simplicity First:** Make every change as simple as possible. Affect minimal code. +- **No Laziness:** Find root causes. No temporary fixes. Senior developer standards. +- **Minimal Impact:** Changes should only touch what's necessary. Avoid introducing bugs. +- **No Backward Compatibility:** Never add backward-compatibility shims, deprecated code paths, or migration logic. Just make the change directly. From 36d709d0ad63f93be3c8edce8d2550d657d1e2ef Mon Sep 17 00:00:00 2001 From: Marti Date: Tue, 14 Apr 2026 15:09:30 +0000 Subject: [PATCH 12/20] chore: remove spaces from hook matches --- .claude/settings.json | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/.claude/settings.json b/.claude/settings.json index d45e33fa5d..509dd32209 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -6,23 +6,23 @@ "hooks": [ { "type": "command", - "if": "Bash(* git *commit *)", + "if": "Bash(*git *commit*)", "command": ".claude/hooks/pre-commit-lint.sh" }, { "type": "command", - "if": "Bash(* git push *)", + "if": "Bash(*git push*)", "command": ".claude/hooks/pre-push-test.sh" }, { "type": "agent", - "if": "Bash(* git push *)", + "if": "Bash(*git push*)", "prompt": "Analyze the commit history on this branch since it diverged from its base. Find the fork point with: `BASE=$(git merge-base HEAD @{u} 2>/dev/null || git merge-base HEAD next)`, then `git log --oneline $BASE..HEAD`. Look for: (1) debug/WIP/fixup commits that should be squashed or dropped (2) commits with unhelpful messages like 'fix', 'wip', 'tmp'. If cleanup is needed, perform an interactive-style rebase using `git rebase` to squash, drop, or reword commits. Use `git -c user.name='Claude (Opus)' -c user.email='noreply@anthropic.com' -c commit.gpgsign=false` for any new commits. If the history already looks clean, do nothing.", "model": "sonnet" }, { "type": "command", - "if": "Bash(* git push *)", + "if": "Bash(*git push*)", "command": ".claude/hooks/pre-push-review.sh" } ] @@ -34,7 +34,7 @@ "hooks": [ { "type": "command", - "if": "Bash(* gh pr create *)", + "if": "Bash(*gh pr create*)", "command": ".claude/hooks/post-pr-create-ci-monitor.sh" } ] From c58cc5b60ff2fc2b3d30b883ea6ec726c741065c Mon Sep 17 00:00:00 2001 From: "Claude (Opus)" Date: Tue, 14 Apr 2026 22:35:03 +0000 Subject: [PATCH 13/20] fix: harden hooks without over-engineering - Stop forcing reviewers to fabricate findings; nits/notes are advisory - Require explicit APPROVE:/CLEAN: verdict (not just absence of BLOCK:) - Use repo root in lint/test hooks so they work from subdirectories - Add branch guards to rebase agent (skip protected branches) - Pass --allowedTools to review agents for Bash/Read access - Add per-project lessons link to CLAUDE.md - Remove dangling .claude/skills symlink Co-Authored-By: Claude Opus 4.6 (1M context) fix: CI monitor needs --allowedTools and effort: medium to actually execute --- .claude/CLAUDE.md | 2 ++ .claude/agents/ci-monitor.md | 4 +-- .claude/agents/code-reviewer.md | 6 ++-- .claude/agents/security-reviewer.md | 10 +++--- .claude/hooks/post-pr-create-ci-monitor.sh | 5 ++- .claude/hooks/pre-commit-lint.sh | 11 ++++-- .claude/hooks/pre-push-review.sh | 41 +++++++++++++++++----- .claude/hooks/pre-push-test.sh | 14 ++++++-- .claude/settings.json | 17 --------- .claude/skills | 1 - 10 files changed, 68 insertions(+), 43 deletions(-) delete mode 120000 .claude/skills diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md index 2dd56c1fd2..186187bdc7 100644 --- a/.claude/CLAUDE.md +++ b/.claude/CLAUDE.md @@ -1,5 +1,7 @@ # CLAUDE.md +> Per-project lessons: ~/.claude/projects/protocol/lessons.md + ## Workflow Orchestration ### 1. Plan Mode Default diff --git a/.claude/agents/ci-monitor.md b/.claude/agents/ci-monitor.md index 2ac12f00e6..ee95515582 100644 --- a/.claude/agents/ci-monitor.md +++ b/.claude/agents/ci-monitor.md @@ -2,7 +2,7 @@ name: ci-monitor description: Waits for CI to run, then checks status, diagnoses failures, and pushes fixes. Spawned in background after PR creation. model: haiku -effort: low +effort: medium tools: Read, Grep, Glob, Bash, Edit, Write maxTurns: 30 --- @@ -47,7 +47,7 @@ If the failure is fixable: 5. Commit and push: ```bash git add - git -c user.name="Claude (Opus)" -c user.email="noreply@anthropic.com" commit -m "fix: " + git -c user.name="Claude (Opus)" -c user.email="noreply@anthropic.com" -c commit.gpgsign=false commit -m "fix: " git push ``` diff --git a/.claude/agents/code-reviewer.md b/.claude/agents/code-reviewer.md index 969659e905..ab9356af39 100644 --- a/.claude/agents/code-reviewer.md +++ b/.claude/agents/code-reviewer.md @@ -104,7 +104,7 @@ Categorize every finding: 6. Be direct. "This will panic when the vec is empty" not "this might possibly be a concern" 7. New code without tests is always a finding -**Every finding - including Nits - blocks the merge.** Minor improvements must be addressed before merging, not left as tech debt. +**Critical and Important findings block the merge.** Nits are advisory and do not block. -If you find any issues at any severity level, start your final response with `BLOCK:` followed by the review. -If the changes pass review with zero findings, start your final response with `APPROVE:` followed by the review. +If you find any Critical or Important issues, start your final response with `BLOCK:` followed by the review. +If there are no Critical or Important issues (only Nits or no findings), start your final response with `APPROVE:` followed by the review. diff --git a/.claude/agents/security-reviewer.md b/.claude/agents/security-reviewer.md index 35df38c136..5a1dfe8f6b 100644 --- a/.claude/agents/security-reviewer.md +++ b/.claude/agents/security-reviewer.md @@ -19,7 +19,7 @@ For every file in the diff, read the **full file**. Vulnerabilities hide in how ## Step 2: Run Both Personas -Execute each persona sequentially. Each persona **MUST** produce at least one finding. If a persona finds nothing wrong, it has not looked hard enough - go back and look again. +Execute each persona sequentially. Each persona should look thoroughly - if it finds nothing after careful examination, note that explicitly rather than fabricating findings. Do not soften findings. Do not hedge. Either it's a problem or it isn't. Be direct. @@ -97,15 +97,15 @@ After both personas report: [2-3 sentences: overall risk profile and the single most important thing to fix] ``` -**Every finding - including Notes - blocks the merge.** Minor improvements must be addressed before merging, not left as tech debt. +**Critical Findings and Warnings block the merge.** Notes are advisory and do not block. **Verdicts:** -- **BLOCK** - Any findings at any severity level. Do not merge until addressed. -- **CLEAN** - Zero findings. Safe to merge. +- **BLOCK** - Any Critical Findings or Warnings. Do not merge until addressed. +- **CLEAN** - No Critical Findings or Warnings (Notes are acceptable). Safe to merge. ## Anti-Patterns - Do NOT Do These -- **"LGTM, no issues found"** - Every change has at least one risk. If you found nothing, you didn't look hard enough. +- **"LGTM, no issues found"** - Be skeptical if you found nothing, but don't fabricate findings. If a change is genuinely clean, use the CLEAN verdict. - **Pulling punches** - "This might possibly be a minor concern" is useless. Say what's wrong. - **Restating the diff** - "This function was added" is not a finding. What's WRONG with it? - **Cosmetic-only findings** - Reporting style issues while missing a panic is worse than no review. diff --git a/.claude/hooks/post-pr-create-ci-monitor.sh b/.claude/hooks/post-pr-create-ci-monitor.sh index 3e5fa79986..4ffb7c8021 100755 --- a/.claude/hooks/post-pr-create-ci-monitor.sh +++ b/.claude/hooks/post-pr-create-ci-monitor.sh @@ -27,7 +27,10 @@ fi LOG_FILE="/tmp/ci-monitor-pr-${PR_NUMBER}.log" -nohup bash -c "cd $CWD && claude --agent ci-monitor -p 'Wait 20 minutes (sleep 1200), then check CI status for PR #${PR_NUMBER} (${PR_URL}). If any checks failed, diagnose and fix them. If all checks pass, just confirm and exit.'" > "$LOG_FILE" 2>&1 & +export CI_MON_CWD="$CWD" +export CI_MON_PR="$PR_NUMBER" +export CI_MON_URL="$PR_URL" +nohup bash -c 'cd "$CI_MON_CWD" && claude --agent ci-monitor --allowedTools "Bash Read Grep Glob Edit Write" -p "Wait 20 minutes (sleep 1200), then check CI status for PR #$CI_MON_PR ($CI_MON_URL). If any checks failed, diagnose and fix them. If all checks pass, just confirm and exit."' > "$LOG_FILE" 2>&1 & echo "CI monitor spawned for PR #${PR_NUMBER} (PID: $!, will check in ~20min, log: ${LOG_FILE})" exit 0 diff --git a/.claude/hooks/pre-commit-lint.sh b/.claude/hooks/pre-commit-lint.sh index 67e3cdf34a..c502058de7 100755 --- a/.claude/hooks/pre-commit-lint.sh +++ b/.claude/hooks/pre-commit-lint.sh @@ -2,17 +2,22 @@ # Pre-commit hook: runs `make lint` in Rust repositories before allowing git commit. # Exit 0 = allow, Exit 2 = block (reason on stderr). +REPO_ROOT=$(git rev-parse --show-toplevel 2>/dev/null) +if [ -z "$REPO_ROOT" ]; then + exit 0 +fi + # Only act in Rust repositories -if [ ! -f "Cargo.toml" ]; then +if [ ! -f "$REPO_ROOT/Cargo.toml" ]; then exit 0 fi # Check that a Makefile with a lint target exists -if ! grep -q '^lint' Makefile 2>/dev/null; then +if ! grep -q '^lint' "$REPO_ROOT/Makefile" 2>/dev/null; then exit 0 fi -OUTPUT=$(make lint 2>&1) +OUTPUT=$(make -C "$REPO_ROOT" lint 2>&1) STATUS=$? if [ $STATUS -ne 0 ]; then diff --git a/.claude/hooks/pre-push-review.sh b/.claude/hooks/pre-push-review.sh index bcea327c6d..1095410825 100755 --- a/.claude/hooks/pre-push-review.sh +++ b/.claude/hooks/pre-push-review.sh @@ -3,24 +3,30 @@ # Both must pass for the push to proceed. # Exit 0 = allow, Exit 2 = block (reason on stderr). -BASE=$(git merge-base HEAD @{u} 2>/dev/null || git merge-base HEAD next) -DIFF=$(git diff "${BASE}...HEAD" 2>/dev/null) +BASE=$(git merge-base HEAD @{u} 2>/dev/null || git merge-base HEAD next 2>/dev/null) -if [ -z "$DIFF" ]; then +if [ -z "$BASE" ]; then + echo "Review blocked: could not determine base branch." >&2 + exit 2 +fi + +# Skip review if there are no changes +if git diff --quiet "$BASE" HEAD; then exit 0 fi PROMPT="Review the changes about to be pushed." +ALLOWED_TOOLS="Bash(git:*) Read Grep Glob" # Run both reviewers in parallel CODE_RESULT_FILE=$(mktemp) SEC_RESULT_FILE=$(mktemp) trap 'rm -f "$CODE_RESULT_FILE" "$SEC_RESULT_FILE"' EXIT -claude --agent code-reviewer -p "$PROMPT" > "$CODE_RESULT_FILE" 2>&1 & +claude --agent code-reviewer --allowedTools "$ALLOWED_TOOLS" -p "$PROMPT" > "$CODE_RESULT_FILE" 2>/dev/null & PID_CODE=$! -claude --agent security-reviewer -p "$PROMPT" > "$SEC_RESULT_FILE" 2>&1 & +claude --agent security-reviewer --allowedTools "$ALLOWED_TOOLS" -p "$PROMPT" > "$SEC_RESULT_FILE" 2>/dev/null & PID_SEC=$! wait $PID_CODE @@ -29,16 +35,20 @@ wait $PID_SEC CODE_RESULT=$(cat "$CODE_RESULT_FILE") SEC_RESULT=$(cat "$SEC_RESULT_FILE") +# Find verdict line (first line starting with BLOCK:/APPROVE:/CLEAN:) +CODE_VERDICT=$(grep -m1 -E '^(BLOCK:|APPROVE:|CLEAN:)' "$CODE_RESULT_FILE" || true) +SEC_VERDICT=$(grep -m1 -E '^(BLOCK:|APPROVE:|CLEAN:)' "$SEC_RESULT_FILE" || true) + BLOCKED=0 -if echo "$CODE_RESULT" | grep -q "^BLOCK:"; then +if [[ "$CODE_VERDICT" == BLOCK:* ]]; then echo "=== CODE REVIEWER: BLOCKED ===" >&2 echo "$CODE_RESULT" >&2 echo "" >&2 BLOCKED=1 fi -if echo "$SEC_RESULT" | grep -q "^BLOCK:"; then +if [[ "$SEC_VERDICT" == BLOCK:* ]]; then echo "=== SECURITY REVIEWER: BLOCKED ===" >&2 echo "$SEC_RESULT" >&2 echo "" >&2 @@ -49,11 +59,24 @@ if [ $BLOCKED -eq 1 ]; then exit 2 fi +# Require explicit approval from both reviewers +if [[ "$CODE_VERDICT" != APPROVE:* ]]; then + echo "Review blocked: code reviewer did not produce APPROVE: verdict." >&2 + echo "$CODE_RESULT" >&2 + exit 2 +fi + +if [[ "$SEC_VERDICT" != APPROVE:* ]] && [[ "$SEC_VERDICT" != CLEAN:* ]]; then + echo "Review blocked: security reviewer did not produce APPROVE:/CLEAN: verdict." >&2 + echo "$SEC_RESULT" >&2 + exit 2 +fi + # Print approvals for visibility -echo "=== CODE REVIEWER: APPROVED ===" >&2 +echo "=== CODE REVIEWER ===" >&2 echo "$CODE_RESULT" >&2 echo "" >&2 -echo "=== SECURITY REVIEWER: CLEAN ===" >&2 +echo "=== SECURITY REVIEWER ===" >&2 echo "$SEC_RESULT" >&2 exit 0 diff --git a/.claude/hooks/pre-push-test.sh b/.claude/hooks/pre-push-test.sh index 49bf39cfb3..1590fff793 100755 --- a/.claude/hooks/pre-push-test.sh +++ b/.claude/hooks/pre-push-test.sh @@ -2,13 +2,23 @@ # Pre-push hook: runs `make test` before allowing push. # Exit 0 = allow, Exit 2 = block (reason on stderr). +REPO_ROOT=$(git rev-parse --show-toplevel 2>/dev/null) +if [ -z "$REPO_ROOT" ]; then + exit 0 +fi + # Only act in Rust repositories -if [ ! -f "Cargo.toml" ]; then +if [ ! -f "$REPO_ROOT/Cargo.toml" ]; then + exit 0 +fi + +# Check that a Makefile with a test target exists +if ! grep -q '^test' "$REPO_ROOT/Makefile" 2>/dev/null; then exit 0 fi echo "Running make test..." >&2 -OUTPUT=$(make test 2>&1) +OUTPUT=$(make -C "$REPO_ROOT" test 2>&1) STATUS=$? if [ $STATUS -ne 0 ]; then diff --git a/.claude/settings.json b/.claude/settings.json index 509dd32209..65ba7da3ff 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -14,12 +14,6 @@ "if": "Bash(*git push*)", "command": ".claude/hooks/pre-push-test.sh" }, - { - "type": "agent", - "if": "Bash(*git push*)", - "prompt": "Analyze the commit history on this branch since it diverged from its base. Find the fork point with: `BASE=$(git merge-base HEAD @{u} 2>/dev/null || git merge-base HEAD next)`, then `git log --oneline $BASE..HEAD`. Look for: (1) debug/WIP/fixup commits that should be squashed or dropped (2) commits with unhelpful messages like 'fix', 'wip', 'tmp'. If cleanup is needed, perform an interactive-style rebase using `git rebase` to squash, drop, or reword commits. Use `git -c user.name='Claude (Opus)' -c user.email='noreply@anthropic.com' -c commit.gpgsign=false` for any new commits. If the history already looks clean, do nothing.", - "model": "sonnet" - }, { "type": "command", "if": "Bash(*git push*)", @@ -39,17 +33,6 @@ } ] } - ], - "TaskCompleted": [ - { - "hooks": [ - { - "type": "agent", - "prompt": "If there are uncommitted changes: review the diff. If all files belong, stage them by name and commit with a descriptive message using: git -c user.name='Claude (Opus)' -c user.email='noreply@anthropic.com' -c commit.gpgsign=false commit -m 'message'. If any files look out of place (build artifacts, generated files, unrelated changes), abort and explain which files are problematic.", - "model": "haiku" - } - ] - } ] } } diff --git a/.claude/skills b/.claude/skills deleted file mode 120000 index 6838a11606..0000000000 --- a/.claude/skills +++ /dev/null @@ -1 +0,0 @@ -../.ai/skills \ No newline at end of file From bc649c3e2c4440c731ca53717e5e11cc26adb1c6 Mon Sep 17 00:00:00 2001 From: Marti Date: Wed, 15 Apr 2026 19:38:17 +0000 Subject: [PATCH 14/20] chore: add a note to Claude.md on commiting frequently --- .claude/CLAUDE.md | 1 + 1 file changed, 1 insertion(+) diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md index 186187bdc7..7021a83d29 100644 --- a/.claude/CLAUDE.md +++ b/.claude/CLAUDE.md @@ -39,6 +39,7 @@ - **Worktrees:** Always work in a git worktree when possible (use `EnterWorktree` with a descriptive name for the feature). This allows parallel agents to work in the same repo without conflicts. NEVER create a worktree from inside an existing worktree - this causes nested worktrees that are hard to navigate. If you are already in a worktree, just work there directly. - **Worktree visibility:** Always tell the user which worktree (full path) you will work in as part of the plan. When finished, state where the changes live (worktree path and branch name). - **Commit authorship:** Always commit as Claude, not as the user. Use: `git -c user.name="Claude (Opus)" -c user.email="noreply@anthropic.com" -c commit.gpgsign=false commit -m "message"` +- **Commit frequency:** Always commit at the end of each task. Avoid single commits that span multiple unrelated changes. ## Output Formatting From e40f790418573a093d1dff15083cf2f17dd104e5 Mon Sep 17 00:00:00 2001 From: Marti Date: Wed, 15 Apr 2026 20:18:20 +0000 Subject: [PATCH 15/20] chore: remove CI monitor hook --- .claude/agents/ci-monitor.md | 63 ------------------------------------ .claude/settings.json | 12 ------- 2 files changed, 75 deletions(-) delete mode 100644 .claude/agents/ci-monitor.md diff --git a/.claude/agents/ci-monitor.md b/.claude/agents/ci-monitor.md deleted file mode 100644 index ee95515582..0000000000 --- a/.claude/agents/ci-monitor.md +++ /dev/null @@ -1,63 +0,0 @@ ---- -name: ci-monitor -description: Waits for CI to run, then checks status, diagnoses failures, and pushes fixes. Spawned in background after PR creation. -model: haiku -effort: medium -tools: Read, Grep, Glob, Bash, Edit, Write -maxTurns: 30 ---- - -# CI Monitor - -You check a PR's CI status, diagnose failures, and push fixes. You run autonomously - the user is not watching. - -## Input - -You will be given a PR number or URL. - -## Workflow - -### 1. Check CI Status - -```bash -gh pr checks -``` - -If all checks pass, report success and exit. - -If checks are still running, wait a few minutes and check again (up to 3 retries). - -### 2. Diagnose Failures - -For each failed check: -1. Get the failed job's logs: - ```bash - gh run view --log-failed - ``` -2. Identify the root cause from the logs -3. Determine if this is a code issue you can fix (not infra/flaky) - -### 3. Fix and Push - -If the failure is fixable: -1. Read the relevant source files -2. Make the fix -3. Run `make lint` to ensure the fix is clean -4. Run relevant tests locally if possible -5. Commit and push: - ```bash - git add - git -c user.name="Claude (Opus)" -c user.email="noreply@anthropic.com" -c commit.gpgsign=false commit -m "fix: " - git push - ``` - -If the failure is NOT fixable (infra issue, flaky test, external dependency): -- Log what you found and exit - -## Rules - -1. **Don't break things.** If unsure about a fix, skip it. A bad fix is worse than a failing CI. -2. **One commit per fix.** Don't batch unrelated fixes. -3. **Always run `make lint` before committing.** No exceptions. -4. **Never force push.** -5. **Log your progress.** Print what you're doing so the output log is useful. diff --git a/.claude/settings.json b/.claude/settings.json index 65ba7da3ff..db57f2cda9 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -21,18 +21,6 @@ } ] } - ], - "PostToolUse": [ - { - "matcher": "Bash", - "hooks": [ - { - "type": "command", - "if": "Bash(*gh pr create*)", - "command": ".claude/hooks/post-pr-create-ci-monitor.sh" - } - ] - } ] } } From adf99d78feff7037e5539010352498cb1dd618a1 Mon Sep 17 00:00:00 2001 From: Marti Date: Wed, 15 Apr 2026 20:30:57 +0000 Subject: [PATCH 16/20] chore: deny non-draft PRs --- .claude/hooks/pre-pr-draft.sh | 20 ++++++++++++++++++++ .claude/settings.json | 4 ++++ 2 files changed, 24 insertions(+) create mode 100755 .claude/hooks/pre-pr-draft.sh diff --git a/.claude/hooks/pre-pr-draft.sh b/.claude/hooks/pre-pr-draft.sh new file mode 100755 index 0000000000..27c409b4c5 --- /dev/null +++ b/.claude/hooks/pre-pr-draft.sh @@ -0,0 +1,20 @@ +#!/bin/bash +# PreToolUse hook: deny gh pr create if --draft is missing + +INPUT=$(cat) +COMMAND=$(echo "$INPUT" | jq -r '.tool_input.command // empty') + +# Only act on gh pr create commands +if ! echo "$COMMAND" | grep -q "gh pr create"; then + exit 0 +fi + +# Allow if --draft is present +if echo "$COMMAND" | grep -q "\-\-draft"; then + exit 0 +fi + +# Deny: missing --draft +cat <<'EOF' +{"hookSpecificOutput":{"hookEventName":"PreToolUse","permissionDecision":"deny","permissionDecisionReason":"All PRs must be created as drafts. Add --draft to the command."}} +EOF diff --git a/.claude/settings.json b/.claude/settings.json index db57f2cda9..1764e5578b 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -18,6 +18,10 @@ "type": "command", "if": "Bash(*git push*)", "command": ".claude/hooks/pre-push-review.sh" + }, + { + "type": "command", + "command": ".claude/hooks/pre-pr-draft.sh" } ] } From 842c630150ced16c063ced35bcb9880cc699f300 Mon Sep 17 00:00:00 2001 From: Marti Date: Thu, 16 Apr 2026 09:04:30 +0000 Subject: [PATCH 17/20] chore: make both reviewers block on nits --- .claude/agents/code-reviewer.md | 6 +++--- .claude/agents/security-reviewer.md | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.claude/agents/code-reviewer.md b/.claude/agents/code-reviewer.md index ab9356af39..1aecb31c96 100644 --- a/.claude/agents/code-reviewer.md +++ b/.claude/agents/code-reviewer.md @@ -104,7 +104,7 @@ Categorize every finding: 6. Be direct. "This will panic when the vec is empty" not "this might possibly be a concern" 7. New code without tests is always a finding -**Critical and Important findings block the merge.** Nits are advisory and do not block. +**All findings (Critical, Important, and Nit) block the merge.** Every issue must be addressed before pushing. -If you find any Critical or Important issues, start your final response with `BLOCK:` followed by the review. -If there are no Critical or Important issues (only Nits or no findings), start your final response with `APPROVE:` followed by the review. +If you find any issues at any severity level, start your final response with `BLOCK:` followed by the review. +If there are zero findings, start your final response with `APPROVE:` followed by the review. diff --git a/.claude/agents/security-reviewer.md b/.claude/agents/security-reviewer.md index 5a1dfe8f6b..fecd527ea4 100644 --- a/.claude/agents/security-reviewer.md +++ b/.claude/agents/security-reviewer.md @@ -97,11 +97,11 @@ After both personas report: [2-3 sentences: overall risk profile and the single most important thing to fix] ``` -**Critical Findings and Warnings block the merge.** Notes are advisory and do not block. +**All findings (Critical, Warning, and Note) block the merge.** Every issue must be addressed before pushing. **Verdicts:** -- **BLOCK** - Any Critical Findings or Warnings. Do not merge until addressed. -- **CLEAN** - No Critical Findings or Warnings (Notes are acceptable). Safe to merge. +- **BLOCK** - Any findings at any severity level. Do not merge until addressed. +- **CLEAN** - Zero findings. Safe to merge. ## Anti-Patterns - Do NOT Do These From 2e7bd1cadf5da147031161132c09ebe4bce79137 Mon Sep 17 00:00:00 2001 From: "Claude (Opus)" Date: Thu, 16 Apr 2026 11:52:46 +0000 Subject: [PATCH 18/20] feat: add post-PR changelog hook Adds a PostToolUse hook that fires after PR creation and spawns a changelog-manager agent to classify the diff. The agent determines whether a CHANGELOG.md entry or "no changelog" label is needed, and feeds actionable instructions back to the main agent via exit code 2. Co-Authored-By: Claude Opus 4.6 (1M context) --- .claude/agents/changelog-manager.md | 98 +++++++++++++++++++++++ .claude/hooks/post-pr-create-changelog.sh | 62 ++++++++++++++ .claude/settings.json | 12 +++ 3 files changed, 172 insertions(+) create mode 100644 .claude/agents/changelog-manager.md create mode 100755 .claude/hooks/post-pr-create-changelog.sh diff --git a/.claude/agents/changelog-manager.md b/.claude/agents/changelog-manager.md new file mode 100644 index 0000000000..b8379c5de2 --- /dev/null +++ b/.claude/agents/changelog-manager.md @@ -0,0 +1,98 @@ +--- +name: changelog-manager +description: Read-only agent that classifies PR diffs and determines whether a CHANGELOG.md entry or "no changelog" label is needed. Spawned automatically after PR creation. +model: sonnet +tools: Bash, Read, Grep, Glob +maxTurns: 5 +--- + +# Changelog Manager + +You are a read-only agent that classifies PR diffs to determine whether a CHANGELOG.md entry is needed. You do NOT modify any files, commit, or apply labels - you only analyze and output a verdict. + +## Input + +You receive a prompt like: `Check changelog for PR #N (URL)` + +## Step 1: Check if Already Handled + +1. Check if the PR already has the `no changelog` label: + ``` + gh pr view --json labels --jq '.labels[].name' + ``` +2. Check if CHANGELOG.md is already modified in the diff: + ``` + git diff origin/next...HEAD -- CHANGELOG.md + ``` + +If either condition is met, output `SKIP: already handled` and stop. + +## Step 2: Analyze the Diff + +Run: +``` +git diff origin/next...HEAD -- ':(exclude)CHANGELOG.md' +``` + +## Step 3: Classify + +**No changelog needed** (output `NO_CHANGELOG: `) - only if ALL changed files fall into these categories: +- Documentation-only changes (README, docs/, comments) +- CI/CD changes (.github/, scripts/) +- Test-only changes (no src/ changes) +- Config/tooling changes (.claude/, .gitignore, Makefile, Cargo.toml metadata) +- Typo or formatting fixes with no behavioral change + +If even one file falls outside the above categories and affects runtime behavior, a changelog entry IS needed. + +**Changelog needed** (output `CHANGELOG: ...`): +- Any changes under src/ or lib/ that affect runtime behavior +- New features, bug fixes, breaking changes +- Changes to MASM files that affect behavior +- New or modified public API surface +- Dependency version bumps that affect behavior + +## Step 4: Output Verdict + +Your output MUST start with exactly one of these verdict lines: + +### SKIP +``` +SKIP: already handled +``` + +### NO_CHANGELOG +``` +NO_CHANGELOG: +``` + +### CHANGELOG +``` +CHANGELOG: +- Entry text ([#N](url)). +``` + +Where `` is one of: `### Features`, `### Changes`, `### Fixes` + +## Entry Format Rules + +Follow the exact style from CHANGELOG.md: +- Past-tense verb: "Added", "Fixed", "Changed", "Removed" +- Prefix `[BREAKING] ` if the change breaks public API +- Use backticks for code identifiers (types, functions, modules) +- One sentence, concise +- End with PR link: `([#N](https://github.com/0xMiden/protocol/pull/N))` +- End with a period after the closing parenthesis + +Example: +``` +CHANGELOG: ### Changes +- Added `AssetAmount` wrapper type for validated fungible asset amounts ([#2721](https://github.com/0xMiden/protocol/pull/2721)). +``` + +## Rules + +1. You are READ-ONLY. Never modify files, commit, or apply labels. +2. The verdict line MUST be the very first line of your final output. +3. When in doubt, prefer requiring a changelog entry (let the human decide to skip). +4. For mixed changes (src/ + docs), a changelog entry is needed. diff --git a/.claude/hooks/post-pr-create-changelog.sh b/.claude/hooks/post-pr-create-changelog.sh new file mode 100755 index 0000000000..cf663fe70e --- /dev/null +++ b/.claude/hooks/post-pr-create-changelog.sh @@ -0,0 +1,62 @@ +#!/bin/bash +# Post-PR-create hook: spawns a changelog-manager agent to check whether +# a CHANGELOG.md entry or "no changelog" label is needed. +# Outputs actionable instructions to the main agent via hookSpecificOutput. + +INPUT=$(cat) + +# Extract PR URL from the tool response +PR_URL=$(echo "$INPUT" | jq -r '.tool_response // empty' | grep -oP 'https://github\.com/[^\s"]+/pull/\d+' | head -1) + +if [ -z "$PR_URL" ]; then + exit 0 +fi + +# Extract PR number +PR_NUMBER=$(echo "$PR_URL" | grep -oP '\d+$') + +if [ -z "$PR_NUMBER" ]; then + exit 0 +fi + +# Extract repo working directory +CWD=$(echo "$INPUT" | jq -r '.cwd // empty') + +if [ -z "$CWD" ]; then + exit 0 +fi + +PROMPT="Check changelog for PR #${PR_NUMBER} (${PR_URL}). Important: if the diff contains ANY changes that affect runtime behavior, a changelog entry is needed - even if the PR also contains config/tooling/docs changes." +ALLOWED_TOOLS="Bash(git:*) Bash(gh:*) Read Grep Glob" + +RESULT_FILE=$(mktemp) +trap 'rm -f "$RESULT_FILE"' EXIT + +cd "$CWD" && claude --agent changelog-manager --allowedTools "$ALLOWED_TOOLS" -p "$PROMPT" > "$RESULT_FILE" 2>/dev/null + +VERDICT=$(grep -m1 -E '^(SKIP:|NO_CHANGELOG:|CHANGELOG:)' "$RESULT_FILE" || true) + +if [[ "$VERDICT" == SKIP:* ]]; then + exit 0 +fi + +if [[ "$VERDICT" == NO_CHANGELOG:* ]]; then + cat < Date: Thu, 16 Apr 2026 12:33:55 +0000 Subject: [PATCH 19/20] chore: try to make the agent be concise in changelogs --- .claude/agents/changelog-manager.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.claude/agents/changelog-manager.md b/.claude/agents/changelog-manager.md index b8379c5de2..83ebebd209 100644 --- a/.claude/agents/changelog-manager.md +++ b/.claude/agents/changelog-manager.md @@ -80,7 +80,7 @@ Follow the exact style from CHANGELOG.md: - Past-tense verb: "Added", "Fixed", "Changed", "Removed" - Prefix `[BREAKING] ` if the change breaks public API - Use backticks for code identifiers (types, functions, modules) -- One sentence, concise +- One short sentence - be succinct, not descriptive - End with PR link: `([#N](https://github.com/0xMiden/protocol/pull/N))` - End with a period after the closing parenthesis From 15bd4e7f31963da908ad69c48208d72cbc2a40de Mon Sep 17 00:00:00 2001 From: Dominik Schmid <35031754+Dominik1999@users.noreply.github.com> Date: Wed, 6 May 2026 09:34:43 +0200 Subject: [PATCH 20/20] Improve Claude setup based on NTL battle-testing (#2847) * Improve Claude setup based on NTL battle-testing Consolidated pre-push hook: - Merged pre-push-test.sh into pre-push-review.sh (tests run first, then reviewers only if tests pass - saves reviewer tokens on broken code) - Severity-based blocking: nits and notes don't block push, only Critical/Important/Warning findings do - Proper diff base resolution: tries upstream, falls back to default branch via gh, then HEAD~1 - Handles reviewer crashes gracefully (empty/malformed output = block) - Escape hatch: SKIP_PRE_PUSH=1 Input guards on all hooks: - Each hook validates the command matches before acting (defense in depth against settings.json filter regressions) Improved settings.json: - Removed separate pre-push-test.sh entry (now in pre-push-review.sh) - Added proper `if` filter on pre-pr-draft.sh (was firing on all Bash) - Removed post-pr-create-ci-monitor.sh (superseded) Lessons system: - Added .claude/lessons.md for codifying review feedback - Added .claude/commands/codify-lesson.md slash command - Updated CLAUDE.md to reference in-repo lessons file instead of global ~/.claude path Agent improvements: - Code reviewer and security reviewer use gh-based default branch resolution instead of hardcoded branch names Co-Authored-By: Claude Opus 4.6 (1M context) * Address review: remove guards, split hooks, drop lessons - Remove input guards from all hooks (boilerplate, unclear value) - Restore pre-push-test.sh as separate hook (keep tests and reviews conceptually separate per review feedback) - Remove lessons.md and codify-lesson.md (use SKILLS.md instead) - Revert CLAUDE.md lessons reference - Keep: severity-based blocking, diff base resolution, crash handling, proper if filters, agent branch resolution via gh Co-Authored-By: Claude Opus 4.6 (1M context) * Adding new lines at the end of the file * Removing unecessary code from post-pr-create-changelog.sh * removing code from pre-pr-draft.sh * remove escape hatch from pre-pr-draft.sh * simplifying pre-pr-draft.sh * removing escape hatch and default org repo from pre-push-review.sh --------- Co-authored-by: Claude (Opus) --- .claude/agents/code-reviewer.md | 4 +- .claude/agents/security-reviewer.md | 4 +- .claude/hooks/post-pr-create-changelog.sh | 96 +++++++----- .claude/hooks/post-pr-create-ci-monitor.sh | 36 ----- .claude/hooks/pre-pr-draft.sh | 39 +++-- .claude/hooks/pre-push-review.sh | 172 ++++++++++++++------- .claude/settings.json | 1 + 7 files changed, 210 insertions(+), 142 deletions(-) delete mode 100755 .claude/hooks/post-pr-create-ci-monitor.sh diff --git a/.claude/agents/code-reviewer.md b/.claude/agents/code-reviewer.md index 1aecb31c96..f912df21d0 100644 --- a/.claude/agents/code-reviewer.md +++ b/.claude/agents/code-reviewer.md @@ -13,7 +13,9 @@ You are an experienced Staff Engineer conducting a thorough code review with fre ## Step 1: Gather Context -Run `git diff @{upstream}...HEAD` (fall back to `git diff next...HEAD` if no upstream is set). +Run `git diff @{upstream}...HEAD`. If no upstream is set, resolve the default +branch with `gh repo view --json defaultBranchRef --jq '.defaultBranchRef.name'` +and run `git diff origin/...HEAD`. For every file in the diff, read the **full file** - not just the changed lines. Bugs hide in how new code interacts with existing code. diff --git a/.claude/agents/security-reviewer.md b/.claude/agents/security-reviewer.md index fecd527ea4..cd1b4a0cbd 100644 --- a/.claude/agents/security-reviewer.md +++ b/.claude/agents/security-reviewer.md @@ -13,7 +13,9 @@ You are a hostile reviewer. Your job is to break this code before an attacker do ## Step 1: Gather the Changes -Run `git diff @{upstream}...HEAD` (fall back to `git diff next...HEAD` if no upstream is set). +Run `git diff @{upstream}...HEAD`. If no upstream is set, resolve the default +branch with `gh repo view --json defaultBranchRef --jq '.defaultBranchRef.name'` +and run `git diff origin/...HEAD`. For every file in the diff, read the **full file**. Vulnerabilities hide in how new code interacts with existing code, not just in the diff itself. diff --git a/.claude/hooks/post-pr-create-changelog.sh b/.claude/hooks/post-pr-create-changelog.sh index cf663fe70e..e65dc24e10 100755 --- a/.claude/hooks/post-pr-create-changelog.sh +++ b/.claude/hooks/post-pr-create-changelog.sh @@ -1,62 +1,88 @@ #!/bin/bash -# Post-PR-create hook: spawns a changelog-manager agent to check whether -# a CHANGELOG.md entry or "no changelog" label is needed. +# Post-PR-create hook: spawns a changelog-manager agent to classify the PR diff +# and decide whether a CHANGELOG.md entry or "no changelog" label is needed. # Outputs actionable instructions to the main agent via hookSpecificOutput. +# +# Wiring (in .claude/settings.json): +# { +# "type": "command", +# "if": "Bash(*gh pr create*)", +# "command": ".claude/hooks/post-pr-create-changelog.sh" +# } +# +# The agent is responsible for locating the correct unreleased section in +# CHANGELOG.md. This hook does not pre-resolve a version. -INPUT=$(cat) - -# Extract PR URL from the tool response -PR_URL=$(echo "$INPUT" | jq -r '.tool_response // empty' | grep -oP 'https://github\.com/[^\s"]+/pull/\d+' | head -1) - -if [ -z "$PR_URL" ]; then - exit 0 -fi - -# Extract PR number -PR_NUMBER=$(echo "$PR_URL" | grep -oP '\d+$') +set -uo pipefail -if [ -z "$PR_NUMBER" ]; then - exit 0 -fi +INPUT=$(cat) -# Extract repo working directory -CWD=$(echo "$INPUT" | jq -r '.cwd // empty') +PR_URL=$(printf '%s' "$INPUT" | jq -r '.tool_response // empty' \ + | grep -oE 'https://github\.com/[^\s"]+/pull/[0-9]+' | head -1) +PR_NUMBER=$(printf '%s' "$PR_URL" | grep -oE '[0-9]+$') +CWD=$(printf '%s' "$INPUT" | jq -r '.cwd // empty') -if [ -z "$CWD" ]; then - exit 0 -fi +[ -z "$PR_URL" ] || [ -z "$PR_NUMBER" ] || [ -z "$CWD" ] && exit 0 -PROMPT="Check changelog for PR #${PR_NUMBER} (${PR_URL}). Important: if the diff contains ANY changes that affect runtime behavior, a changelog entry is needed - even if the PR also contains config/tooling/docs changes." +# ---------------------------------------------------------------------------- +# Spawn the classifier agent. +# ---------------------------------------------------------------------------- +PROMPT="Check changelog for PR #${PR_NUMBER} (${PR_URL}). Important: if the diff contains ANY changes that affect runtime behavior, a changelog entry is needed, even if the PR also contains config/tooling/docs changes." ALLOWED_TOOLS="Bash(git:*) Bash(gh:*) Read Grep Glob" RESULT_FILE=$(mktemp) -trap 'rm -f "$RESULT_FILE"' EXIT +trap 'rm -f "$RESULT_FILE" "$RESULT_FILE.err"' EXIT -cd "$CWD" && claude --agent changelog-manager --allowedTools "$ALLOWED_TOOLS" -p "$PROMPT" > "$RESULT_FILE" 2>/dev/null +cd "$CWD" && claude --agent changelog-manager --allowedTools "$ALLOWED_TOOLS" -p "$PROMPT" > "$RESULT_FILE" 2> "$RESULT_FILE.err" VERDICT=$(grep -m1 -E '^(SKIP:|NO_CHANGELOG:|CHANGELOG:)' "$RESULT_FILE" || true) +# ---------------------------------------------------------------------------- +# Dispatch on verdict. +# ---------------------------------------------------------------------------- +emit_context() { + # Wrap a free-form message into a valid PostToolUse JSON payload. + printf '%s' "$1" | jq -Rs '{hookSpecificOutput:{hookEventName:"PostToolUse",additionalContext:.}}' +} + if [[ "$VERDICT" == SKIP:* ]]; then exit 0 fi if [[ "$VERDICT" == NO_CHANGELOG:* ]]; then - cat < "$LOG_FILE" 2>&1 & - -echo "CI monitor spawned for PR #${PR_NUMBER} (PID: $!, will check in ~20min, log: ${LOG_FILE})" -exit 0 diff --git a/.claude/hooks/pre-pr-draft.sh b/.claude/hooks/pre-pr-draft.sh index 27c409b4c5..5be17ec9ea 100755 --- a/.claude/hooks/pre-pr-draft.sh +++ b/.claude/hooks/pre-pr-draft.sh @@ -1,20 +1,39 @@ #!/bin/bash -# PreToolUse hook: deny gh pr create if --draft is missing +# PreToolUse hook for the Bash tool: blocks `gh pr create` invocations that +# do not pass --draft. PRs must be created as drafts; a human promotes them +# to ready-for-review when appropriate. +# +# Wiring (in .claude/settings.json): +# { +# "type": "command", +# "if": "Bash(*gh pr create*)", +# "command": ".claude/hooks/pre-pr-draft.sh" +# } +# +# Output protocol: writes JSON to stdout per the Claude Code PreToolUse hook +# contract. Exit code is always 0; the deny signal is carried in the JSON +# payload's `permissionDecision` field. +set -uo pipefail + +# Read the hook input. Fail open on malformed input so the hook can never +# wedge tool use in a bad state. INPUT=$(cat) -COMMAND=$(echo "$INPUT" | jq -r '.tool_input.command // empty') +COMMAND=$(printf '%s' "$INPUT" | jq -r '.tool_input.command // empty' 2>/dev/null || true) -# Only act on gh pr create commands -if ! echo "$COMMAND" | grep -q "gh pr create"; then +if [ -z "$COMMAND" ]; then exit 0 fi -# Allow if --draft is present -if echo "$COMMAND" | grep -q "\-\-draft"; then +# Allow if --draft is already present. +if printf '%s' "$COMMAND" | grep -qE '(^|[[:space:]])--draft([[:space:]=]|$)'; then exit 0 fi -# Deny: missing --draft -cat <<'EOF' -{"hookSpecificOutput":{"hookEventName":"PreToolUse","permissionDecision":"deny","permissionDecisionReason":"All PRs must be created as drafts. Add --draft to the command."}} -EOF +# Otherwise deny, with a corrected command. +REASON=$(printf 'PRs must be created as drafts. Re-run with --draft:\n\n %s --draft' "$COMMAND") +REASON_JSON=$(printf '%s' "$REASON" | jq -Rs .) + +printf '{"hookSpecificOutput":{"hookEventName":"PreToolUse","permissionDecision":"deny","permissionDecisionReason":%s}}\n' "$REASON_JSON" + +exit 0 diff --git a/.claude/hooks/pre-push-review.sh b/.claude/hooks/pre-push-review.sh index 1095410825..baa45fecce 100755 --- a/.claude/hooks/pre-push-review.sh +++ b/.claude/hooks/pre-push-review.sh @@ -1,82 +1,136 @@ #!/bin/bash -# Pre-push hook: spawns two independent review agents before allowing push. -# Both must pass for the push to proceed. -# Exit 0 = allow, Exit 2 = block (reason on stderr). +# Pre-push hook: spawns code-reviewer + security-reviewer in parallel. +# Blocks the push on (a) any Critical/Important/Warning finding from +# either reviewer, or (b) reviewer crash or malformed output. +# Nits and Notes are surfaced to the user but never block. +# +# Severity policy (single source of truth, not the agent prompts): +# BLOCK on ### Critical Issues | ### Critical Findings +# ### Important Issues | ### Warnings +# IGNORE ### Nits | ### Notes | ### What's Done Well | ### Summary + +set -uo pipefail + +REPO_ROOT=$(git rev-parse --show-toplevel 2>/dev/null || true) +if [ -z "$REPO_ROOT" ]; then + echo "Pre-push: not inside a git worktree, skipping." >&2 + exit 0 +fi -BASE=$(git merge-base HEAD @{u} 2>/dev/null || git merge-base HEAD next 2>/dev/null) +# Determine the diff base. Prefer the configured upstream; fall back to +# origin/next (the repo's default branch). +BASE="" +if UPSTREAM=$(git rev-parse --abbrev-ref --symbolic-full-name @{u} 2>/dev/null); then + BASE="$UPSTREAM" +else + BASE="origin/next" +fi -if [ -z "$BASE" ]; then - echo "Review blocked: could not determine base branch." >&2 - exit 2 +MERGE_BASE=$(git merge-base HEAD "$BASE" 2>/dev/null || git rev-parse HEAD~1 2>/dev/null || true) +if [ -z "$MERGE_BASE" ]; then + echo "Pre-push: cannot resolve merge-base against $BASE; allowing." >&2 + exit 0 fi -# Skip review if there are no changes -if git diff --quiet "$BASE" HEAD; then +if git diff --quiet "$MERGE_BASE" HEAD; then + echo "Pre-push: no changes vs $BASE; skipping." >&2 exit 0 fi -PROMPT="Review the changes about to be pushed." +TMPDIR=$(mktemp -d) +trap 'rm -rf "$TMPDIR"' EXIT +CODE_OUT="$TMPDIR/code.out" +SEC_OUT="$TMPDIR/sec.out" + +# ---------------------------------------------------------------------------- +# Reviewers (parallel). +# ---------------------------------------------------------------------------- +PROMPT="Review the changes about to be pushed (diff base: ${MERGE_BASE})." ALLOWED_TOOLS="Bash(git:*) Read Grep Glob" -# Run both reviewers in parallel -CODE_RESULT_FILE=$(mktemp) -SEC_RESULT_FILE=$(mktemp) -trap 'rm -f "$CODE_RESULT_FILE" "$SEC_RESULT_FILE"' EXIT +echo "Pre-push: spawning code-reviewer + security-reviewer..." >&2 -claude --agent code-reviewer --allowedTools "$ALLOWED_TOOLS" -p "$PROMPT" > "$CODE_RESULT_FILE" 2>/dev/null & +claude --agent code-reviewer --allowedTools "$ALLOWED_TOOLS" -p "$PROMPT" > "$CODE_OUT" 2> "$TMPDIR/code.err" & PID_CODE=$! - -claude --agent security-reviewer --allowedTools "$ALLOWED_TOOLS" -p "$PROMPT" > "$SEC_RESULT_FILE" 2>/dev/null & +claude --agent security-reviewer --allowedTools "$ALLOWED_TOOLS" -p "$PROMPT" > "$SEC_OUT" 2> "$TMPDIR/sec.err" & PID_SEC=$! -wait $PID_CODE -wait $PID_SEC - -CODE_RESULT=$(cat "$CODE_RESULT_FILE") -SEC_RESULT=$(cat "$SEC_RESULT_FILE") - -# Find verdict line (first line starting with BLOCK:/APPROVE:/CLEAN:) -CODE_VERDICT=$(grep -m1 -E '^(BLOCK:|APPROVE:|CLEAN:)' "$CODE_RESULT_FILE" || true) -SEC_VERDICT=$(grep -m1 -E '^(BLOCK:|APPROVE:|CLEAN:)' "$SEC_RESULT_FILE" || true) - -BLOCKED=0 - -if [[ "$CODE_VERDICT" == BLOCK:* ]]; then - echo "=== CODE REVIEWER: BLOCKED ===" >&2 - echo "$CODE_RESULT" >&2 +wait $PID_CODE; RC_CODE=$? +wait $PID_SEC; RC_SEC=$? + +# ---------------------------------------------------------------------------- +# Parse findings. Block ONLY on Critical/Important/Warning sections. +# Nits and Notes are surfaced via the report dump but ignored for the +# blocking decision. +# ---------------------------------------------------------------------------- + +count_blocking_findings() { + awk ' + BEGIN { in_block = 0; count = 0 } + /^##[^#]|^## / { in_block = 0 } + /^### / { + if ($0 ~ /^### (Critical|Important|Warnings)([[:space:]]|$)/) { + in_block = 1 + } else { + in_block = 0 + } + next + } + in_block && /^[[:space:]]*[-*][[:space:]]+./ { count++ } + END { print count } + ' "$1" +} + +review_is_valid() { + [ -s "$1" ] && grep -q '^### ' "$1" +} + +evaluate_reviewer() { + local name="$1" rc="$2" out="$3" echo "" >&2 - BLOCKED=1 -fi - -if [[ "$SEC_VERDICT" == BLOCK:* ]]; then - echo "=== SECURITY REVIEWER: BLOCKED ===" >&2 - echo "$SEC_RESULT" >&2 + echo "=== ${name} ===" >&2 + + if [ "$rc" -ne 0 ]; then + echo "${name}: agent exited with status ${rc}; treating as block." >&2 + [ -s "$out" ] && cat "$out" >&2 + local err_file="" + case "$name" in + "CODE REVIEWER") err_file="$TMPDIR/code.err" ;; + "SECURITY REVIEWER") err_file="$TMPDIR/sec.err" ;; + esac + [ -n "$err_file" ] && [ -s "$err_file" ] && { echo "--- agent stderr ---" >&2; cat "$err_file" >&2; } + return 1 + fi + + if ! review_is_valid "$out"; then + echo "${name}: empty or malformed output; treating as block." >&2 + [ -s "$out" ] && cat "$out" >&2 + return 1 + fi + + cat "$out" >&2 + + local n + n=$(count_blocking_findings "$out") echo "" >&2 - BLOCKED=1 -fi - -if [ $BLOCKED -eq 1 ]; then - exit 2 -fi + if [ "$n" -gt 0 ]; then + echo "${name}: ${n} blocking finding(s) (Critical/Important/Warning)." >&2 + return 1 + fi + echo "${name}: no blocking findings (nits/notes do not block)." >&2 + return 0 +} -# Require explicit approval from both reviewers -if [[ "$CODE_VERDICT" != APPROVE:* ]]; then - echo "Review blocked: code reviewer did not produce APPROVE: verdict." >&2 - echo "$CODE_RESULT" >&2 - exit 2 -fi +BLOCKED=0 +evaluate_reviewer "CODE REVIEWER" "$RC_CODE" "$CODE_OUT" || BLOCKED=1 +evaluate_reviewer "SECURITY REVIEWER" "$RC_SEC" "$SEC_OUT" || BLOCKED=1 -if [[ "$SEC_VERDICT" != APPROVE:* ]] && [[ "$SEC_VERDICT" != CLEAN:* ]]; then - echo "Review blocked: security reviewer did not produce APPROVE:/CLEAN: verdict." >&2 - echo "$SEC_RESULT" >&2 +if [ "$BLOCKED" -eq 1 ]; then + echo "" >&2 + echo "Pre-push: push blocked. Address Critical/Important/Warning findings above and retry." >&2 exit 2 fi -# Print approvals for visibility -echo "=== CODE REVIEWER ===" >&2 -echo "$CODE_RESULT" >&2 echo "" >&2 -echo "=== SECURITY REVIEWER ===" >&2 -echo "$SEC_RESULT" >&2 - +echo "Pre-push: all checks passed." >&2 exit 0 diff --git a/.claude/settings.json b/.claude/settings.json index a943cd5bd6..e12a293c15 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -21,6 +21,7 @@ }, { "type": "command", + "if": "Bash(*gh pr create*)", "command": ".claude/hooks/pre-pr-draft.sh" } ]