From 0e6e23caa6d8b573003d951055da2963ce5cdc6a Mon Sep 17 00:00:00 2001 From: Dominik Schmid Date: Wed, 22 Apr 2026 13:12:04 +0200 Subject: [PATCH 01/10] Claude-first setup --- .claude/CLAUDE.md | 57 ++++++++++ .claude/agents/changelog-manager.md | 98 ++++++++++++++++ .claude/agents/code-reviewer.md | 110 ++++++++++++++++++ .claude/agents/security-reviewer.md | 124 +++++++++++++++++++++ .claude/hooks/post-pr-create-changelog.sh | 62 +++++++++++ .claude/hooks/post-pr-create-ci-monitor.sh | 36 ++++++ .claude/hooks/pre-commit-lint.sh | 29 +++++ .claude/hooks/pre-pr-draft.sh | 20 ++++ .claude/hooks/pre-push-review.sh | 82 ++++++++++++++ .claude/hooks/pre-push-test.sh | 31 ++++++ .claude/settings.json | 42 +++++++ .gitignore | 4 +- 12 files changed, 694 insertions(+), 1 deletion(-) create mode 100644 .claude/CLAUDE.md create mode 100644 .claude/agents/changelog-manager.md create mode 100644 .claude/agents/code-reviewer.md create mode 100644 .claude/agents/security-reviewer.md create mode 100644 .claude/hooks/post-pr-create-changelog.sh create mode 100644 .claude/hooks/post-pr-create-ci-monitor.sh create mode 100644 .claude/hooks/pre-commit-lint.sh create mode 100644 .claude/hooks/pre-pr-draft.sh create mode 100644 .claude/hooks/pre-push-review.sh create mode 100644 .claude/hooks/pre-push-test.sh create mode 100644 .claude/settings.json diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md new file mode 100644 index 0000000..6773672 --- /dev/null +++ b/.claude/CLAUDE.md @@ -0,0 +1,57 @@ +# CLAUDE.md + +> Per-project lessons: ~/.claude/projects/note-transport/lessons.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. `dominik1999-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"` +- **Commit frequency:** Always commit at the end of each task. Avoid single commits that span multiple unrelated changes. + +## 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. \ No newline at end of file diff --git a/.claude/agents/changelog-manager.md b/.claude/agents/changelog-manager.md new file mode 100644 index 0000000..ccd1185 --- /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 short sentence - be succinct, not descriptive +- End with PR link: `([#N](https://github.com/0xMiden/note-transport-service/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/note-transport-service/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. \ No newline at end of file diff --git a/.claude/agents/code-reviewer.md b/.claude/agents/code-reviewer.md new file mode 100644 index 0000000..596de96 --- /dev/null +++ b/.claude/agents/code-reviewer.md @@ -0,0 +1,110 @@ +--- +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 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. + +## 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 + +**All findings (Critical, Important, and Nit) block the merge.** Every issue must be addressed before pushing. + +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. \ No newline at end of file diff --git a/.claude/agents/security-reviewer.md b/.claude/agents/security-reviewer.md new file mode 100644 index 0000000..da7ef6c --- /dev/null +++ b/.claude/agents/security-reviewer.md @@ -0,0 +1,124 @@ +--- +name: security-reviewer +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 +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 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. + +## Step 2: Run Both Personas + +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. + +### Persona 1: The Adversary + +**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? +- Could an authenticated caller escalate privileges through this? + +Look for: +- 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 +- Integer overflow/underflow, off-by-one errors, unchecked arithmetic +- Panics/unwraps in non-test code +- Resource leaks (handles, connections, allocations) +- Hardcoded credentials, secrets in code/config/comments +- 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 2: The Auditor + +**Mindset:** "I must certify this code meets its own safety invariants." + +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 +- 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 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 invariants. Should fix before merge. + +**NOTE** - Minor improvement opportunity or fragile assumption worth documenting. + +## Output Format + +``` +## Adversarial Security Review + +**Verdict:** BLOCK | 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] +``` + +**All findings (Critical, Warning, and Note) block the merge.** Every issue must be addressed before pushing. + +**Verdicts:** +- **BLOCK** - Any findings at any severity level. Do not merge until addressed. +- **CLEAN** - Zero findings. Safe to merge. + +## Anti-Patterns - Do NOT Do These + +- **"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. +- **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 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. \ No newline at end of file diff --git a/.claude/hooks/post-pr-create-changelog.sh b/.claude/hooks/post-pr-create-changelog.sh new file mode 100644 index 0000000..e985abc --- /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 < "$LOG_FILE" 2>&1 & + +echo "CI monitor spawned for PR #${PR_NUMBER} (PID: $!, will check in ~20min, log: ${LOG_FILE})" +exit 0 \ No newline at end of file diff --git a/.claude/hooks/pre-commit-lint.sh b/.claude/hooks/pre-commit-lint.sh new file mode 100644 index 0000000..a10b568 --- /dev/null +++ b/.claude/hooks/pre-commit-lint.sh @@ -0,0 +1,29 @@ +#!/bin/bash +# 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 "$REPO_ROOT/Cargo.toml" ]; then + exit 0 +fi + +# Check that a Makefile with a lint target exists +if ! grep -q '^lint' "$REPO_ROOT/Makefile" 2>/dev/null; then + exit 0 +fi + +OUTPUT=$(make -C "$REPO_ROOT" 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 \ No newline at end of file diff --git a/.claude/hooks/pre-pr-draft.sh b/.claude/hooks/pre-pr-draft.sh new file mode 100644 index 0000000..8670025 --- /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 \ No newline at end of file diff --git a/.claude/hooks/pre-push-review.sh b/.claude/hooks/pre-push-review.sh new file mode 100644 index 0000000..9f2d934 --- /dev/null +++ b/.claude/hooks/pre-push-review.sh @@ -0,0 +1,82 @@ +# !/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). + +BASE=$(git merge-base HEAD @{u} 2>/dev/null || git merge-base HEAD next 2>/dev/null) + +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 --allowedTools "$ALLOWED_TOOLS" -p "$PROMPT" > "$CODE_RESULT_FILE" 2>/dev/null & +PID_CODE=$! + +claude --agent security-reviewer --allowedTools "$ALLOWED_TOOLS" -p "$PROMPT" > "$SEC_RESULT_FILE" 2>/dev/null & +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 + echo "" >&2 + BLOCKED=1 +fi + +if [[ "$SEC_VERDICT" == BLOCK:* ]]; then + echo "=== SECURITY REVIEWER: BLOCKED ===" >&2 + echo "$SEC_RESULT" >&2 + echo "" >&2 + BLOCKED=1 +fi + +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 ===" >&2 +echo "$CODE_RESULT" >&2 +echo "" >&2 +echo "=== SECURITY REVIEWER ===" >&2 +echo "$SEC_RESULT" >&2 + +exit 0 \ No newline at end of file diff --git a/.claude/hooks/pre-push-test.sh b/.claude/hooks/pre-push-test.sh new file mode 100644 index 0000000..734fe24 --- /dev/null +++ b/.claude/hooks/pre-push-test.sh @@ -0,0 +1,31 @@ +# !/bin/bash +# 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 "$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 -C "$REPO_ROOT" 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 \ No newline at end of file diff --git a/.claude/settings.json b/.claude/settings.json new file mode 100644 index 0000000..c25c01d --- /dev/null +++ b/.claude/settings.json @@ -0,0 +1,42 @@ +{ + "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-test.sh" + }, + { + "type": "command", + "if": "Bash(*git push*)", + "command": ".claude/hooks/pre-push-review.sh" + }, + { + "type": "command", + "command": ".claude/hooks/pre-pr-draft.sh" + } + ] + } + ], + "PostToolUse": [ + { + "matcher": "Bash", + "hooks": [ + { + "type": "command", + "if": "Bash(*gh pr create*)", + "command": ".claude/hooks/post-pr-create-changelog.sh" + } + ] + } + ] + } +} \ No newline at end of file diff --git a/.gitignore b/.gitignore index cf3d763..d113dbd 100644 --- a/.gitignore +++ b/.gitignore @@ -9,7 +9,9 @@ *.masl # Ignore Claude Code config -.claude/ +# Ignore Claude Code local config and worktrees +.claude/settings.local.json +.claude/worktrees/ # Docs platform ignores .vscode From 5704cb74fb59de3079ac56a8d2b187ac55b60d40 Mon Sep 17 00:00:00 2001 From: Dominik Schmid Date: Wed, 22 Apr 2026 14:17:32 +0200 Subject: [PATCH 02/10] some fixes --- .claude/agents/code-reviewer.md | 2 +- .claude/agents/security-reviewer.md | 4 +- .claude/hooks/post-pr-create-changelog.sh | 2 +- .claude/hooks/pre-pr-draft.sh | 83 +++++++-- .claude/hooks/pre-push-review.sh | 216 ++++++++++++++++------ .claude/settings.json | 46 +++-- 6 files changed, 254 insertions(+), 99 deletions(-) diff --git a/.claude/agents/code-reviewer.md b/.claude/agents/code-reviewer.md index 596de96..8e55c33 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 -**All findings (Critical, Important, and Nit) block the merge.** Every issue must be addressed before pushing. +**Findings (Critical, Important) block the merge.** Every issue must be addressed before pushing. 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. \ No newline at end of file diff --git a/.claude/agents/security-reviewer.md b/.claude/agents/security-reviewer.md index da7ef6c..eab959a 100644 --- a/.claude/agents/security-reviewer.md +++ b/.claude/agents/security-reviewer.md @@ -97,7 +97,7 @@ After both personas report: [2-3 sentences: overall risk profile and the single most important thing to fix] ``` -**All findings (Critical, Warning, and Note) block the merge.** Every issue must be addressed before pushing. +**All critical or warning findings block the merge.** Every issue must be addressed before pushing. **Verdicts:** - **BLOCK** - Any findings at any severity level. Do not merge until addressed. @@ -121,4 +121,4 @@ You may share the same mental model as the code's author. To break this: 5. Ask: "If I deleted this change entirely, what would break?" If nothing, the change might be unnecessary. 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. \ No newline at end of file +If there are zero critical or important findings, start with `CLEAN:` followed by the review. \ No newline at end of file diff --git a/.claude/hooks/post-pr-create-changelog.sh b/.claude/hooks/post-pr-create-changelog.sh index e985abc..61f93f5 100644 --- a/.claude/hooks/post-pr-create-changelog.sh +++ b/.claude/hooks/post-pr-create-changelog.sh @@ -53,7 +53,7 @@ if [[ "$VERDICT" == CHANGELOG:* ]]; then # Escape for JSON ENTRY_ESCAPED=$(echo "$ENTRY" | python3 -c 'import sys,json; print(json.dumps(sys.stdin.read())[1:-1])') cat <` once human review is requested. +# +# 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. +# +# Escape hatch: set SKIP_DRAFT_CHECK=1 in the environment to bypass. + +set -uo pipefail + +# Escape hatch. +if [ "${SKIP_DRAFT_CHECK:-}" = "1" ]; then + exit 0 +fi + +# 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') - -# Only act on gh pr create commands -if ! echo "$COMMAND" | grep -q "gh pr create"; then +COMMAND=$(printf '%s' "$INPUT" | jq -r '.tool_input.command // empty' 2>/dev/null || true) + +if [ -z "$COMMAND" ]; then exit 0 fi - -# Allow if --draft is present -if echo "$COMMAND" | grep -q "\-\-draft"; then + +# Defensive command match. The settings.json `if` filter should already +# restrict us to `gh pr create`, but we re-check in case the hook is wired +# without a filter or invoked directly. The leading boundary excludes +# false matches like `mygh pr create`; quotes and other shell glue are +# permitted before `gh` so `bash -c "gh pr create ..."` still matches. +if ! printf '%s' "$COMMAND" | grep -qE '(^|[^a-zA-Z0-9_-])gh[[:space:]]+pr[[:space:]]+create'; 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 \ No newline at end of file + +# Decision: --no-draft (anywhere) wins over --draft. This is stricter than +# gh's last-wins flag parsing, which is intentional for a deny-by-default +# hook: if the user typed --no-draft they meant it, even if --draft also +# slipped in elsewhere. +if printf '%s' "$COMMAND" | grep -qE '(^|[[:space:]])--no-draft([[:space:]=]|$)'; then + : # explicit opt-out, fall through to deny +elif printf '%s' "$COMMAND" | grep -qE '(^|[[:space:]])--draft([[:space:]=]|$)'; then + exit 0 +fi + +# Build a corrected command suggestion: strip any --no-draft (flag form or +# `--no-draft=value` form, but never consume the next argument), collapse +# the resulting whitespace, then append --draft if not already present. +# Best-effort only; a literal "--no-draft" inside a quoted title would be +# mangled, but the user can edit. +SUGGESTED=$(printf '%s' "$COMMAND" \ + | sed -E 's/(^|[[:space:]])--no-draft(=[^[:space:]]*)?([[:space:]]|$)/\1\3/g' \ + | sed -E 's/[[:space:]]+/ /g' \ + | sed -E 's/[[:space:]]+$//') + +if ! printf '%s' "$SUGGESTED" | grep -qE '(^|[[:space:]])--draft([[:space:]=]|$)'; then + SUGGESTED="${SUGGESTED} --draft" +fi + +REASON=$(printf 'PRs must be created as drafts. Re-run with --draft:\n\n %s\n\nPromote to ready-for-review later with: gh pr ready \nBypass this hook with: SKIP_DRAFT_CHECK=1 gh pr create ...' "$SUGGESTED") + +# JSON-encode the reason string (yields a quoted, escaped JSON string). +REASON_JSON=$(printf '%s' "$REASON" | jq -Rs .) + +printf '{"hookSpecificOutput":{"hookEventName":"PreToolUse","permissionDecision":"deny","permissionDecisionReason":%s}}\n' "$REASON_JSON" + +exit 0 \ No newline at end of file diff --git a/.claude/hooks/pre-push-review.sh b/.claude/hooks/pre-push-review.sh index 9f2d934..2d80b43 100644 --- a/.claude/hooks/pre-push-review.sh +++ b/.claude/hooks/pre-push-review.sh @@ -1,82 +1,186 @@ -# !/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). +#!/bin/bash +# Pre-push hook: runs tests, then spawns code-reviewer + security-reviewer in +# parallel. Blocks the push on (a) test failure, (b) any Critical/Important/ +# Warning finding from either reviewer, or (c) reviewer crash or malformed +# output. Nits and Notes are surfaced to the user but never block. +# +# This script is the single pre-push entry point. Remove the pre-push-test +# hook entry from .claude/settings.json so tests don't run twice. Wire as: +# +# { +# "type": "command", +# "if": "Bash(*git push*)", +# "command": ".claude/hooks/pre-push-review.sh" +# } +# +# Output protocol: status and reviewer reports go to stderr (so they surface +# to Claude on exit 2 and don't pollute stdout). Exit 0 = allow push, +# Exit 2 = block push. +# +# 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 +# This intentionally diverges from the "all findings block" line in the +# reviewer prompts. If you want the agents to stop emitting BLOCK: for nits, +# update their prompts; this script doesn't depend on that. +# +# Escape hatch: SKIP_PRE_PUSH=1 bypasses everything. + +set -uo pipefail + +if [ "${SKIP_PRE_PUSH:-}" = "1" ]; then + echo "SKIP_PRE_PUSH=1 set; bypassing pre-push checks." >&2 + exit 0 +fi -BASE=$(git merge-base HEAD @{u} 2>/dev/null || git merge-base HEAD next 2>/dev/null) +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 +# Determine the diff base. Prefer the configured upstream; fall back to +# the repo's default branch resolved via gh; final fallback is HEAD~1. +BASE="" +if UPSTREAM=$(git rev-parse --abbrev-ref --symbolic-full-name @{u} 2>/dev/null); then + BASE="$UPSTREAM" +elif command -v gh >/dev/null 2>&1; then + if DEFAULT=$(gh repo view --json defaultBranchRef --jq '.defaultBranchRef.name' 2>/dev/null); then + [ -n "$DEFAULT" ] && BASE="origin/$DEFAULT" + fi +fi if [ -z "$BASE" ]; then - echo "Review blocked: could not determine base branch." >&2 - exit 2 + echo "Pre-push: cannot determine diff base; falling back to HEAD~1." >&2 + BASE="HEAD~1" fi -# Skip review if there are no changes -if git diff --quiet "$BASE" HEAD; then +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 -PROMPT="Review the changes about to be pushed." +if git diff --quiet "$MERGE_BASE" HEAD; then + echo "Pre-push: no changes vs $BASE; skipping." >&2 + exit 0 +fi + +TMPDIR=$(mktemp -d) +trap 'rm -rf "$TMPDIR"' EXIT +TEST_LOG="$TMPDIR/test.log" +CODE_OUT="$TMPDIR/code.out" +SEC_OUT="$TMPDIR/sec.out" + +# ---------------------------------------------------------------------------- +# Step 1: tests (synchronous, fail-fast). No point burning reviewer tokens +# on changes that don't compile. +# ---------------------------------------------------------------------------- +if [ -f "$REPO_ROOT/Cargo.toml" ] && [ -f "$REPO_ROOT/Makefile" ] \ + && grep -qE '^test:' "$REPO_ROOT/Makefile"; then + echo "Pre-push: running make test..." >&2 + if ! make -C "$REPO_ROOT" test > "$TEST_LOG" 2>&1; then + echo "" >&2 + echo "=== TESTS FAILED ===" >&2 + cat "$TEST_LOG" >&2 + echo "" >&2 + echo "Pre-push: blocked because tests failed. Fix and retry." >&2 + echo "Bypass with: SKIP_PRE_PUSH=1 git push ..." >&2 + exit 2 + fi + echo "Pre-push: tests passed." >&2 +else + echo "Pre-push: no Makefile test target found; skipping tests." >&2 +fi + +# ---------------------------------------------------------------------------- +# Step 2: 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>/dev/null & 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>/dev/null & PID_SEC=$! -wait $PID_CODE -wait $PID_SEC - -CODE_RESULT=$(cat "$CODE_RESULT_FILE") -SEC_RESULT=$(cat "$SEC_RESULT_FILE") +wait $PID_CODE; RC_CODE=$? +wait $PID_SEC; RC_SEC=$? + +# ---------------------------------------------------------------------------- +# Step 3: parse findings. Block ONLY on Critical/Important/Warning sections. +# Nits and Notes are surfaced via the report dump but ignored for the +# blocking decision. +# ---------------------------------------------------------------------------- + +# Counts non-empty bullet lines (- or *) under blocking-severity section +# headers, stopping each section at the next ## or ### header. Prints the +# count to stdout. +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" +} + +# A valid review is non-empty and contains at least one ### header. +review_is_valid() { + [ -s "$1" ] && grep -q '^### ' "$1" +} + +evaluate_reviewer() { + local name="$1" rc="$2" out="$3" + echo "" >&2 + echo "=== ${name} ===" >&2 -# 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) + if [ "$rc" -ne 0 ]; then + echo "${name}: agent exited with status ${rc}; treating as block." >&2 + [ -s "$out" ] && cat "$out" >&2 + return 1 + fi -BLOCKED=0 + if ! review_is_valid "$out"; then + echo "${name}: empty or malformed output; treating as block." >&2 + [ -s "$out" ] && cat "$out" >&2 + return 1 + fi -if [[ "$CODE_VERDICT" == BLOCK:* ]]; then - echo "=== CODE REVIEWER: BLOCKED ===" >&2 - echo "$CODE_RESULT" >&2 - echo "" >&2 - BLOCKED=1 -fi + cat "$out" >&2 -if [[ "$SEC_VERDICT" == BLOCK:* ]]; then - echo "=== SECURITY REVIEWER: BLOCKED ===" >&2 - echo "$SEC_RESULT" >&2 + local n + n=$(count_blocking_findings "$out") echo "" >&2 - BLOCKED=1 -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 +} -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 +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 + echo "Bypass with: SKIP_PRE_PUSH=1 git push ..." >&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 \ No newline at end of file diff --git a/.claude/settings.json b/.claude/settings.json index c25c01d..e2aa027 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -1,31 +1,27 @@ { "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-test.sh" - }, - { - "type": "command", - "if": "Bash(*git push*)", - "command": ".claude/hooks/pre-push-review.sh" - }, - { - "type": "command", - "command": ".claude/hooks/pre-pr-draft.sh" - } - ] - } - ], + { + "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" + }, + { + "type": "command", + "if": "Bash(*gh pr create*)", + "command": ".claude/hooks/pre-pr-draft.sh" + } + ] + } + ], "PostToolUse": [ { "matcher": "Bash", From fd27e9061485d4ba98a237afbfa981e2a047c2f2 Mon Sep 17 00:00:00 2001 From: Dominik Schmid Date: Wed, 22 Apr 2026 14:30:38 +0200 Subject: [PATCH 03/10] some more fixes --- .claude/agents/changelog-manager.md | 4 ++-- .claude/agents/code-reviewer.md | 6 +++--- .claude/agents/security-reviewer.md | 10 +++++----- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/.claude/agents/changelog-manager.md b/.claude/agents/changelog-manager.md index ccd1185..89d42dd 100644 --- a/.claude/agents/changelog-manager.md +++ b/.claude/agents/changelog-manager.md @@ -22,7 +22,7 @@ You receive a prompt like: `Check changelog for PR #N (URL)` ``` 2. Check if CHANGELOG.md is already modified in the diff: ``` - git diff origin/next...HEAD -- CHANGELOG.md + gh repo view --json defaultBranchRef --jq '.defaultBranchRef.name' -- CHANGELOG.md ``` If either condition is met, output `SKIP: already handled` and stop. @@ -31,7 +31,7 @@ If either condition is met, output `SKIP: already handled` and stop. Run: ``` -git diff origin/next...HEAD -- ':(exclude)CHANGELOG.md' +gh repo view --json defaultBranchRef --jq '.defaultBranchRef.name' -- ':(exclude)CHANGELOG.md' ``` ## Step 3: Classify diff --git a/.claude/agents/code-reviewer.md b/.claude/agents/code-reviewer.md index 8e55c33..aa9c865 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 next...HEAD` if no upstream is set). +Run `git diff @{upstream}...HEAD` (fall back to `gh repo view --json defaultBranchRef --jq '.defaultBranchRef.name'` 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. @@ -106,5 +106,5 @@ Categorize every finding: **Findings (Critical, Important) block the merge.** Every issue must be addressed before pushing. -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. \ No newline at end of file +If you find any issues at severity levels Critical or Important, start your final response with `BLOCK:` followed by the review. +If there are zero critical or important findings, start your final response with `APPROVE:` followed by the review. \ No newline at end of file diff --git a/.claude/agents/security-reviewer.md b/.claude/agents/security-reviewer.md index eab959a..102816d 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 next...HEAD` if no upstream is set). +Run `git diff @{upstream}...HEAD` (fall back to `gh repo view --json defaultBranchRef --jq '.defaultBranchRef.name'` 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. @@ -100,8 +100,8 @@ After both personas report: **All critical or warning findings block the merge.** Every issue must be addressed before pushing. **Verdicts:** -- **BLOCK** - Any findings at any severity level. Do not merge until addressed. -- **CLEAN** - Zero findings. Safe to merge. +- **BLOCK** - Any findings at severity levels Critical or Warning. Do not merge until addressed. +- **CLEAN** - Zero critical or important or warning findings. Safe to merge. ## Anti-Patterns - Do NOT Do These @@ -120,5 +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 findings at any severity level, start your final response with `BLOCK:` followed by the review. -If there are zero critical or important findings, start with `CLEAN:` followed by the review. \ No newline at end of file +If you find any findings at severity levels Critical or Important or Warning, start your final response with `BLOCK:` followed by the review. +If there are zero critical or important or warning findings, start with `CLEAN:` followed by the review. \ No newline at end of file From 79a59ae62d746bc591ba5f497e74d0812433b14c Mon Sep 17 00:00:00 2001 From: Dominik Schmid Date: Wed, 22 Apr 2026 14:45:01 +0200 Subject: [PATCH 04/10] adding a Changelog.md --- .claude/CLAUDE.md | 2 +- .claude/commands/codify-lesson.md | 77 +++++++++++++++++++ .claude/hooks/post-pr-create-changelog.sh | 92 ++++++++++++++++------- .claude/lessons.md | 43 +++++++++++ CHANGELOG.md | 52 +++++++++++++ 5 files changed, 238 insertions(+), 28 deletions(-) create mode 100644 .claude/commands/codify-lesson.md create mode 100644 .claude/lessons.md create mode 100644 CHANGELOG.md diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md index 6773672..2aba03c 100644 --- a/.claude/CLAUDE.md +++ b/.claude/CLAUDE.md @@ -1,6 +1,6 @@ # CLAUDE.md -> Per-project lessons: ~/.claude/projects/note-transport/lessons.md +> Per-project lessons: `.claude/lessons.md`. Read at the start of every non-trivial task. Append new entries with `/codify-lesson `. ## Workflow Orchestration diff --git a/.claude/commands/codify-lesson.md b/.claude/commands/codify-lesson.md new file mode 100644 index 0000000..677f67f --- /dev/null +++ b/.claude/commands/codify-lesson.md @@ -0,0 +1,77 @@ +--- +description: Append a lesson learned from a PR review to .claude/lessons.md and propose a promotion path. +argument-hint: +allowed-tools: Read, Edit, Bash(git:*), Bash(gh:*), Bash(date:*) +--- + +You are codifying a lesson from a recent code review or production incident so that future sessions on this project do not repeat the mistake. Be precise and brief. Lessons that take more than six lines to express probably belong in a hook or in CLAUDE.md, not here. + +User input: $ARGUMENTS + +If $ARGUMENTS is empty, ask the user for the lesson and stop. Do not invent one. + +## Steps + +### 1. Determine source + +Run, in this order: + + git rev-parse --short HEAD + git branch --show-current + +If `gh` is available, also run: + + gh pr view --json number,url --jq '"#" + (.number|tostring) + " " + .url' 2>/dev/null + +Use the PR link if one is returned. Otherwise use the short commit SHA. Get the date with `date -u +%Y-%m-%d`. + +### 2. Classify + +Pick exactly one category. Re-read the section headers in `.claude/lessons.md`: + +- **Conventions** — naming, formatting, vocabulary, file layout, doc style +- **Architecture** — module boundaries, abstraction choices, API design +- **Testing** — what to test, fixtures, regression patterns, coverage gaps +- **Security & Safety** — validation, auth, data handling, error paths, panics, resource limits +- **Process** — workflow, commits, PRs, CI, review etiquette, branch naming + +If the lesson genuinely spans two categories, split it into two lessons. Do not file under multiple sections. + +### 3. Format the entry + +Use exactly this template. Each field is one sentence. No em dashes; use `-` instead. + + ### YYYY-MM-DD: + - **Trigger:** + - **Rule:** + - **Why:** + - **Source:** + +### 4. Append to lessons.md + +Read `.claude/lessons.md`. Find the matching `## ` section. Append the entry at the end of that section, immediately before the next `## ` header (or at end of file if it is the last section). If the section currently contains the placeholder `_(no entries yet)_`, replace it with the entry; otherwise add a blank line before the new entry. + +Do not edit any other section. + +### 5. Propose a promotion path + +After the edit, output a recommendation. Pick exactly one and justify in one sentence: + +- **Keep as lesson** — when the rule needs human judgment to apply (e.g., "prefer composition over inheritance for new domain types"). No further action. +- **Promote to hook** — when the rule can be mechanically checked from a script (e.g., "all migrations must include a down.sql"). Draft the hook. If the hook is under 30 lines of bash, write it to `.claude/hooks/.sh` and print the matching `.claude/settings.json` entry as a code block for the user to paste. Do NOT modify settings.json yourself. +- **Promote to CLAUDE.md** — when the rule is a global, always-on instruction (e.g., "never use em dashes"). Print the exact line and the section it belongs in as a unified diff against the current CLAUDE.md. Do NOT apply the edit yourself. + +If you propose promotion to a hook or CLAUDE.md, also note that the lesson should be removed from `lessons.md` once the promotion lands, with a one-line marker like `(promoted to .claude/hooks/foo.sh on YYYY-MM-DD)` left in place. + +### 6. Hand off + +Show the user: +1. The exact text appended to lessons.md +2. The promotion recommendation and any draft hook or CLAUDE.md diff +3. A reminder to commit the change (do not auto-commit; the user may want to bundle it with related work) + +## Constraints + +- Touch only `.claude/lessons.md` without explicit user confirmation. Hooks and CLAUDE.md edits are proposals, not actions. +- Keep entries to the four-bullet template. If the rule needs more nuance, that is the signal to promote it. +- If a near-duplicate of the proposed lesson already exists in lessons.md, point it out and ask whether to merge, supersede, or skip rather than appending a redundant entry. diff --git a/.claude/hooks/post-pr-create-changelog.sh b/.claude/hooks/post-pr-create-changelog.sh index 61f93f5..bca8e1e 100644 --- a/.claude/hooks/post-pr-create-changelog.sh +++ b/.claude/hooks/post-pr-create-changelog.sh @@ -1,31 +1,57 @@ -# !/bin/bash -# Post-PR-create hook: spawns a changelog-manager agent to check whether -# a CHANGELOG.md entry or "no changelog" label is needed. +#!/bin/bash +# 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" +# } +# +# Version resolution: the unreleased section name is read from the PR's +# milestone (or the lowest open milestone) via gh. There is no hardcoded +# fallback because guessing a version is worse than asking the user. + +set -uo pipefail 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) +PR_URL=$(printf '%s' "$INPUT" | jq -r '.tool_response // empty' \ + | grep -oP 'https://github\.com/[^\s"]+/pull/\d+' | head -1) +PR_NUMBER=$(printf '%s' "$PR_URL" | grep -oP '\d+$') +CWD=$(printf '%s' "$INPUT" | jq -r '.cwd // empty') -if [ -z "$PR_URL" ]; then - exit 0 -fi +[ -z "$PR_URL" ] || [ -z "$PR_NUMBER" ] || [ -z "$CWD" ] && exit 0 -# Extract PR number -PR_NUMBER=$(echo "$PR_URL" | grep -oP '\d+$') +# ---------------------------------------------------------------------------- +# Resolve the unreleased version dynamically. Strategy: +# 1. PR's own milestone title (most authoritative) +# 2. lowest open milestone with a version-like title +# 3. give up; tell the user +# ---------------------------------------------------------------------------- +resolve_unreleased_version() { + local pr_number="$1" + local v -if [ -z "$PR_NUMBER" ]; then - exit 0 -fi + if [ -n "$pr_number" ]; then + v=$(gh pr view "$pr_number" --json milestone --jq '.milestone.title // empty' 2>/dev/null \ + | grep -oE '[0-9]+\.[0-9]+\.[0-9]+' | head -1) + [ -n "$v" ] && { printf 'v%s' "$v"; return 0; } + fi -# Extract repo working directory -CWD=$(echo "$INPUT" | jq -r '.cwd // empty') + v=$(gh api 'repos/:owner/:repo/milestones?state=open' --jq '.[].title' 2>/dev/null \ + | grep -oE '[0-9]+\.[0-9]+\.[0-9]+' \ + | sort -V | head -1) + [ -n "$v" ] && { printf 'v%s' "$v"; return 0; } -if [ -z "$CWD" ]; then - exit 0 -fi + return 1 +} +# ---------------------------------------------------------------------------- +# 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" @@ -36,25 +62,37 @@ cd "$CWD" && claude --agent changelog-manager --allowedTools "$ALLOWED_TOOLS" -p 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 < Codified mistakes, conventions, and patterns surfaced during real PR reviews. +> Claude reads this at the start of every non-trivial task (see CLAUDE.md). +> +> Add a new entry with: +> +> /codify-lesson +> +> Each entry must commit to a promotion path: +> +> - **Keep as lesson**: judgment call, context-dependent, hard to mechanize. +> - **Promote to hook**: mechanically enforceable. Lives under `.claude/hooks/`. +> - **Promote to CLAUDE.md**: global project rule that applies to all tasks. +> +> A lesson promoted to a hook or CLAUDE.md should be removed from this file +> with a note like `(promoted to .claude/hooks/foo.sh on YYYY-MM-DD)` so the +> file stays a list of active soft rules, not a graveyard. + +## Conventions +_Naming, formatting, vocabulary, file layout, doc style._ + +_(no entries yet)_ + +## Architecture +_Module boundaries, abstraction choices, API design, dependency direction._ + +_(no entries yet)_ + +## Testing +_What to test, how to test, fixtures, regression patterns, coverage gaps._ + +_(no entries yet)_ + +## Security & Safety +_Validation, auth, data handling, error paths, panics, resource limits._ + +_(no entries yet)_ + +## Process +_Workflow, commits, PRs, CI, review etiquette, branch naming._ + +_(no entries yet)_ diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 0000000..d3d23b6 --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,52 @@ +# Changelog + +All notable user-facing changes to this project are recorded here. + +The format follows the `### Features / ### Changes / ### Fixes` structure expected by +the changelog-manager agent (see `.claude/agents/changelog-manager.md`). This project +adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). + +## How entries are added + +New entries land via the `post-pr-create-changelog` hook (see `.claude/hooks/`). +The unreleased version for any in-flight PR is resolved from the PR's GitHub +milestone, so make sure your PR is assigned to the correct milestone before +opening it. + +Entry style: + +- One past-tense imperative line per change. Start with "Added", "Changed", "Fixed", or "Removed". +- Use backticks for code identifiers (`fetch_notes`, `StoredNote`, `seq`). +- Prefix breaking changes with `[BREAKING] `. +- End with the PR link in parentheses, then a period. + +Example: + +``` +- Fixed `fetch_notes` pagination race by introducing a monotonic `seq` cursor ([#77](https://github.com/0xMiden/note-transport-service/pull/77)). +- [BREAKING] Removed deprecated `fetch_notes_legacy` ([#82](https://github.com/0xMiden/note-transport-service/pull/82)). +``` + +Skip the changelog only when the PR contains no runtime-affecting changes +(docs, CI, tooling, tests). In that case the hook will tell you to apply the +`no changelog` label instead. + +## v0.4.0 (unreleased) + +### Features + +### Changes + +### Fixes + +## v0.3.1 (2026-04-08) + +Released before this changelog was started. See [`git log v0.3.0..v0.3.1`](https://github.com/0xMiden/note-transport-service/compare/v0.3.0...v0.3.1) for the change set. + +## v0.3.0 (2026-04-08) + +Released before this changelog was started. See [`git log v0.2..v0.3.0`](https://github.com/0xMiden/note-transport-service/compare/v0.2...v0.3.0) for the change set. + +## v0.2 (2026-01-24) + +First tagged release. Earlier history available via `git log v0.2`. \ No newline at end of file From 353201aa15d15c2ebc94548564d6230879fdea39 Mon Sep 17 00:00:00 2001 From: Dominik Schmid Date: Wed, 22 Apr 2026 14:52:27 +0200 Subject: [PATCH 05/10] last fixes --- .claude/agents/changelog-manager.md | 2 +- .claude/agents/code-reviewer.md | 4 +++- .claude/agents/security-reviewer.md | 10 ++++++---- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/.claude/agents/changelog-manager.md b/.claude/agents/changelog-manager.md index 89d42dd..017ab12 100644 --- a/.claude/agents/changelog-manager.md +++ b/.claude/agents/changelog-manager.md @@ -87,7 +87,7 @@ Follow the exact style from CHANGELOG.md: Example: ``` CHANGELOG: ### Changes -- Added `AssetAmount` wrapper type for validated fungible asset amounts ([#2721](https://github.com/0xMiden/note-transport-service/pull/2721)). +- fix: close private-note pagination race and :memory: pool-isolation bug ([#77](https://github.com/0xMiden/note-transport-service/pull/77)). ``` ## Rules diff --git a/.claude/agents/code-reviewer.md b/.claude/agents/code-reviewer.md index aa9c865..0b3f17a 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 `gh repo view --json defaultBranchRef --jq '.defaultBranchRef.name'` 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 102816d..ce60377 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 `gh repo view --json defaultBranchRef --jq '.defaultBranchRef.name'` 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. @@ -101,7 +103,7 @@ After both personas report: **Verdicts:** - **BLOCK** - Any findings at severity levels Critical or Warning. Do not merge until addressed. -- **CLEAN** - Zero critical or important or warning findings. Safe to merge. +- **CLEAN** - Zero critical or warning findings. Safe to merge. ## Anti-Patterns - Do NOT Do These @@ -120,5 +122,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 findings at severity levels Critical or Important or Warning, start your final response with `BLOCK:` followed by the review. -If there are zero critical or important or warning findings, start with `CLEAN:` followed by the review. \ No newline at end of file +If you find any findings at severity levels Critical or Warning, start your final response with `BLOCK:` followed by the review. +If there are zero critical or warning findings, start with `CLEAN:` followed by the review. \ No newline at end of file From 207732e19fa4636fffbef9b5d7c694d947b046a4 Mon Sep 17 00:00:00 2001 From: Dominik Schmid Date: Wed, 22 Apr 2026 14:55:39 +0200 Subject: [PATCH 06/10] nit fix --- .claude/agents/changelog-manager.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.claude/agents/changelog-manager.md b/.claude/agents/changelog-manager.md index 017ab12..4274fb5 100644 --- a/.claude/agents/changelog-manager.md +++ b/.claude/agents/changelog-manager.md @@ -86,8 +86,8 @@ Follow the exact style from CHANGELOG.md: Example: ``` -CHANGELOG: ### Changes -- fix: close private-note pagination race and :memory: pool-isolation bug ([#77](https://github.com/0xMiden/note-transport-service/pull/77)). +CHANGELOG: ### Fixes +- Fixed `fetch_notes` pagination race and `:memory:` pool isolation bug ([#77](https://github.com/0xMiden/note-transport-service/pull/77)). ``` ## Rules From 13698abe393edbf4a42072d5c45ac44084572c6c Mon Sep 17 00:00:00 2001 From: Dominik Schmid Date: Wed, 22 Apr 2026 16:02:10 +0200 Subject: [PATCH 07/10] fixes after review --- .claude/agents/changelog-manager.md | 4 +-- .claude/hooks/post-pr-create-changelog.sh | 4 +-- .claude/hooks/post-pr-create-ci-monitor.sh | 36 ---------------------- .claude/hooks/pre-commit-lint.sh | 0 .claude/hooks/pre-pr-draft.sh | 0 .claude/hooks/pre-push-review.sh | 0 .claude/hooks/pre-push-test.sh | 31 ------------------- 7 files changed, 4 insertions(+), 71 deletions(-) mode change 100644 => 100755 .claude/hooks/post-pr-create-changelog.sh delete mode 100644 .claude/hooks/post-pr-create-ci-monitor.sh mode change 100644 => 100755 .claude/hooks/pre-commit-lint.sh mode change 100644 => 100755 .claude/hooks/pre-pr-draft.sh mode change 100644 => 100755 .claude/hooks/pre-push-review.sh delete mode 100644 .claude/hooks/pre-push-test.sh diff --git a/.claude/agents/changelog-manager.md b/.claude/agents/changelog-manager.md index 4274fb5..3e32de2 100644 --- a/.claude/agents/changelog-manager.md +++ b/.claude/agents/changelog-manager.md @@ -22,7 +22,7 @@ You receive a prompt like: `Check changelog for PR #N (URL)` ``` 2. Check if CHANGELOG.md is already modified in the diff: ``` - gh repo view --json defaultBranchRef --jq '.defaultBranchRef.name' -- CHANGELOG.md + git diff origin/...HEAD -- CHANGELOG.md ``` If either condition is met, output `SKIP: already handled` and stop. @@ -31,7 +31,7 @@ If either condition is met, output `SKIP: already handled` and stop. Run: ``` -gh repo view --json defaultBranchRef --jq '.defaultBranchRef.name' -- ':(exclude)CHANGELOG.md' +git diff origin/...HEAD -- ':(exclude)CHANGELOG.md'' ``` ## Step 3: Classify diff --git a/.claude/hooks/post-pr-create-changelog.sh b/.claude/hooks/post-pr-create-changelog.sh old mode 100644 new mode 100755 index bca8e1e..7e0bcf3 --- a/.claude/hooks/post-pr-create-changelog.sh +++ b/.claude/hooks/post-pr-create-changelog.sh @@ -19,8 +19,8 @@ set -uo pipefail INPUT=$(cat) PR_URL=$(printf '%s' "$INPUT" | jq -r '.tool_response // empty' \ - | grep -oP 'https://github\.com/[^\s"]+/pull/\d+' | head -1) -PR_NUMBER=$(printf '%s' "$PR_URL" | grep -oP '\d+$') + | 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') [ -z "$PR_URL" ] || [ -z "$PR_NUMBER" ] || [ -z "$CWD" ] && exit 0 diff --git a/.claude/hooks/post-pr-create-ci-monitor.sh b/.claude/hooks/post-pr-create-ci-monitor.sh deleted file mode 100644 index 9f0cf0f..0000000 --- a/.claude/hooks/post-pr-create-ci-monitor.sh +++ /dev/null @@ -1,36 +0,0 @@ -#!/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" - -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 \ No newline at end of file diff --git a/.claude/hooks/pre-commit-lint.sh b/.claude/hooks/pre-commit-lint.sh old mode 100644 new mode 100755 diff --git a/.claude/hooks/pre-pr-draft.sh b/.claude/hooks/pre-pr-draft.sh old mode 100644 new mode 100755 diff --git a/.claude/hooks/pre-push-review.sh b/.claude/hooks/pre-push-review.sh old mode 100644 new mode 100755 diff --git a/.claude/hooks/pre-push-test.sh b/.claude/hooks/pre-push-test.sh deleted file mode 100644 index 734fe24..0000000 --- a/.claude/hooks/pre-push-test.sh +++ /dev/null @@ -1,31 +0,0 @@ -# !/bin/bash -# 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 "$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 -C "$REPO_ROOT" 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 \ No newline at end of file From 345d74f656b4f79908729c8e51a51cb0043846ff Mon Sep 17 00:00:00 2001 From: Dominik Schmid Date: Wed, 22 Apr 2026 16:29:52 +0200 Subject: [PATCH 08/10] more fixes --- .claude/hooks/post-pr-create-changelog.sh | 9 +++++++++ .claude/hooks/pre-commit-lint.sh | 9 +++++++++ .claude/hooks/pre-pr-draft.sh | 8 ++++++++ .claude/hooks/pre-push-review.sh | 17 +++++++++++++---- 4 files changed, 39 insertions(+), 4 deletions(-) diff --git a/.claude/hooks/post-pr-create-changelog.sh b/.claude/hooks/post-pr-create-changelog.sh index 7e0bcf3..82e411d 100755 --- a/.claude/hooks/post-pr-create-changelog.sh +++ b/.claude/hooks/post-pr-create-changelog.sh @@ -1,4 +1,13 @@ #!/bin/bash +echo "$(date +%H:%M:%S) [post-pr-changelog] fired" >> /tmp/claude-hooks.log + +# Internal guard: only fire for actual git commit invocations. Defense in depth +# against settings.json filter regressions. +COMMAND=$(jq -r '.tool_input.command // empty' 2>/dev/null) +echo "$COMMAND" | grep -qE '(^|[[:space:]])git[[:space:]]+(-c[[:space:]]+[^ ]+[[:space:]]+)*commit([[:space:]]|$)' || exit 0 + +REPO_ROOT=$(git rev-parse --show-toplevel 2>/dev/null) + # 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. diff --git a/.claude/hooks/pre-commit-lint.sh b/.claude/hooks/pre-commit-lint.sh index a10b568..3863832 100755 --- a/.claude/hooks/pre-commit-lint.sh +++ b/.claude/hooks/pre-commit-lint.sh @@ -1,4 +1,13 @@ #!/bin/bash +echo "$(date +%H:%M:%S) [pre-commit-lint] fired" >> /tmp/claude-hooks.log + +# Internal guard: only fire for actual git commit invocations. Defense in depth +# against settings.json filter regressions. +COMMAND=$(jq -r '.tool_input.command // empty' 2>/dev/null) +echo "$COMMAND" | grep -qE '(^|[[:space:]])git[[:space:]]+(-c[[:space:]]+[^ ]+[[:space:]]+)*commit([[:space:]]|$)' || exit 0 + +REPO_ROOT=$(git rev-parse --show-toplevel 2>/dev/null) + # Pre-commit hook: runs `make lint` in Rust repositories before allowing git commit. # Exit 0 = allow, Exit 2 = block (reason on stderr). diff --git a/.claude/hooks/pre-pr-draft.sh b/.claude/hooks/pre-pr-draft.sh index 85f17c8..a0acc4c 100755 --- a/.claude/hooks/pre-pr-draft.sh +++ b/.claude/hooks/pre-pr-draft.sh @@ -1,4 +1,12 @@ #!/bin/bash +echo "$(date +%H:%M:%S) [pre-pr-draft] fired" >> /tmp/claude-hooks.log + +# Internal guard: only fire for actual git commit invocations. Defense in depth +# against settings.json filter regressions. +COMMAND=$(jq -r '.tool_input.command // empty' 2>/dev/null) +echo "$COMMAND" | grep -qE '(^|[[:space:]])git[[:space:]]+(-c[[:space:]]+[^ ]+[[:space:]]+)*commit([[:space:]]|$)' || exit 0 + +REPO_ROOT=$(git rev-parse --show-toplevel 2>/dev/null) # PreToolUse hook for the Bash tool: blocks `gh pr create` invocations that # would create a non-draft PR. Always create PRs as drafts; promote with # `gh pr ready ` once human review is requested. diff --git a/.claude/hooks/pre-push-review.sh b/.claude/hooks/pre-push-review.sh index 2d80b43..9e84e19 100755 --- a/.claude/hooks/pre-push-review.sh +++ b/.claude/hooks/pre-push-review.sh @@ -1,4 +1,12 @@ #!/bin/bash +echo "$(date +%H:%M:%S) [pre-push-review] fired" >> /tmp/claude-hooks.log + +# Internal guard: only fire for actual git commit invocations. Defense in depth +# against settings.json filter regressions. +COMMAND=$(jq -r '.tool_input.command // empty' 2>/dev/null) +echo "$COMMAND" | grep -qE '(^|[[:space:]])git[[:space:]]+(-c[[:space:]]+[^ ]+[[:space:]]+)*commit([[:space:]]|$)' || exit 0 + +REPO_ROOT=$(git rev-parse --show-toplevel 2>/dev/null) # Pre-push hook: runs tests, then spawns code-reviewer + security-reviewer in # parallel. Blocks the push on (a) test failure, (b) any Critical/Important/ # Warning finding from either reviewer, or (c) reviewer crash or malformed @@ -101,9 +109,9 @@ ALLOWED_TOOLS="Bash(git:*) Read Grep Glob" echo "Pre-push: spawning code-reviewer + security-reviewer..." >&2 -claude --agent code-reviewer --allowedTools "$ALLOWED_TOOLS" -p "$PROMPT" > "$CODE_OUT" 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_OUT" 2>/dev/null & +claude --agent security-reviewer --allowedTools "$ALLOWED_TOOLS" -p "$PROMPT" > "$SEC_OUT" 2> "$TMPDIR/sec.err" & PID_SEC=$! wait $PID_CODE; RC_CODE=$? @@ -145,11 +153,12 @@ evaluate_reviewer() { echo "" >&2 echo "=== ${name} ===" >&2 - if [ "$rc" -ne 0 ]; then + if [ "$rc" -ne 0 ]; then echo "${name}: agent exited with status ${rc}; treating as block." >&2 [ -s "$out" ] && cat "$out" >&2 + [ -s "${TMPDIR}/${name_lower}.err" ] && { echo "--- agent stderr ---" >&2; cat "${TMPDIR}/${name_lower}.err" >&2; } return 1 - fi + fi if ! review_is_valid "$out"; then echo "${name}: empty or malformed output; treating as block." >&2 From e04b3161a86b569baf67ebe990a61116ab3423f3 Mon Sep 17 00:00:00 2001 From: Dominik Schmid Date: Wed, 22 Apr 2026 16:40:22 +0200 Subject: [PATCH 09/10] new fixes --- .claude/hooks/post-pr-create-changelog.sh | 33 +++++++++++++++++------ .claude/hooks/pre-commit-lint.sh | 3 --- .claude/hooks/pre-pr-draft.sh | 5 +--- .claude/hooks/pre-push-review.sh | 5 +--- 4 files changed, 27 insertions(+), 19 deletions(-) diff --git a/.claude/hooks/post-pr-create-changelog.sh b/.claude/hooks/post-pr-create-changelog.sh index 82e411d..4c5676f 100755 --- a/.claude/hooks/post-pr-create-changelog.sh +++ b/.claude/hooks/post-pr-create-changelog.sh @@ -1,11 +1,8 @@ #!/bin/bash -echo "$(date +%H:%M:%S) [post-pr-changelog] fired" >> /tmp/claude-hooks.log - # Internal guard: only fire for actual git commit invocations. Defense in depth # against settings.json filter regressions. COMMAND=$(jq -r '.tool_input.command // empty' 2>/dev/null) -echo "$COMMAND" | grep -qE '(^|[[:space:]])git[[:space:]]+(-c[[:space:]]+[^ ]+[[:space:]]+)*commit([[:space:]]|$)' || exit 0 - +echo "$COMMAND" | grep -qE '(^|[^a-zA-Z0-9_-])gh[[:space:]]+pr[[:space:]]+create([[:space:]]|$)' || exit 0 REPO_ROOT=$(git rev-parse --show-toplevel 2>/dev/null) # Post-PR-create hook: spawns a changelog-manager agent to classify the PR diff @@ -65,9 +62,9 @@ PROMPT="Check changelog for PR #${PR_NUMBER} (${PR_URL}). Important: if the diff 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) @@ -105,5 +102,25 @@ ${ENTRY}" exit 2 fi -# No verdict found - fail open, CI will catch it -exit 0 \ No newline at end of file +# No verdict line found. This usually means the classifier agent crashed, +# timed out, or returned output in an unexpected format. Surface the failure +# to the main agent instead of silently exiting so the changelog decision +# isn't skipped without a human knowing. +WARNING="WARNING: changelog-manager produced no verdict for PR #${PR_NUMBER}. Decide manually: add a CHANGELOG.md entry under the appropriate unreleased section, or apply the 'no changelog' label via: gh pr edit ${PR_NUMBER} --add-label 'no changelog'" + +if [ -s "$RESULT_FILE.err" ]; then + WARNING="${WARNING} + +--- classifier stderr --- +$(cat "$RESULT_FILE.err")" +fi + +if [ -s "$RESULT_FILE" ]; then + WARNING="${WARNING} + +--- classifier stdout (no verdict line recognized) --- +$(cat "$RESULT_FILE")" +fi + +emit_context "$WARNING" +exit 2 \ No newline at end of file diff --git a/.claude/hooks/pre-commit-lint.sh b/.claude/hooks/pre-commit-lint.sh index 3863832..238bdfe 100755 --- a/.claude/hooks/pre-commit-lint.sh +++ b/.claude/hooks/pre-commit-lint.sh @@ -1,11 +1,8 @@ #!/bin/bash -echo "$(date +%H:%M:%S) [pre-commit-lint] fired" >> /tmp/claude-hooks.log - # Internal guard: only fire for actual git commit invocations. Defense in depth # against settings.json filter regressions. COMMAND=$(jq -r '.tool_input.command // empty' 2>/dev/null) echo "$COMMAND" | grep -qE '(^|[[:space:]])git[[:space:]]+(-c[[:space:]]+[^ ]+[[:space:]]+)*commit([[:space:]]|$)' || exit 0 - REPO_ROOT=$(git rev-parse --show-toplevel 2>/dev/null) # Pre-commit hook: runs `make lint` in Rust repositories before allowing git commit. diff --git a/.claude/hooks/pre-pr-draft.sh b/.claude/hooks/pre-pr-draft.sh index a0acc4c..e874116 100755 --- a/.claude/hooks/pre-pr-draft.sh +++ b/.claude/hooks/pre-pr-draft.sh @@ -1,11 +1,8 @@ #!/bin/bash -echo "$(date +%H:%M:%S) [pre-pr-draft] fired" >> /tmp/claude-hooks.log - # Internal guard: only fire for actual git commit invocations. Defense in depth # against settings.json filter regressions. COMMAND=$(jq -r '.tool_input.command // empty' 2>/dev/null) -echo "$COMMAND" | grep -qE '(^|[[:space:]])git[[:space:]]+(-c[[:space:]]+[^ ]+[[:space:]]+)*commit([[:space:]]|$)' || exit 0 - +echo "$COMMAND" | grep -qE '(^|[^a-zA-Z0-9_-])gh[[:space:]]+pr[[:space:]]+create([[:space:]]|$)' || exit 0 REPO_ROOT=$(git rev-parse --show-toplevel 2>/dev/null) # PreToolUse hook for the Bash tool: blocks `gh pr create` invocations that # would create a non-draft PR. Always create PRs as drafts; promote with diff --git a/.claude/hooks/pre-push-review.sh b/.claude/hooks/pre-push-review.sh index 9e84e19..d4f42a3 100755 --- a/.claude/hooks/pre-push-review.sh +++ b/.claude/hooks/pre-push-review.sh @@ -1,11 +1,8 @@ #!/bin/bash -echo "$(date +%H:%M:%S) [pre-push-review] fired" >> /tmp/claude-hooks.log - # Internal guard: only fire for actual git commit invocations. Defense in depth # against settings.json filter regressions. COMMAND=$(jq -r '.tool_input.command // empty' 2>/dev/null) -echo "$COMMAND" | grep -qE '(^|[[:space:]])git[[:space:]]+(-c[[:space:]]+[^ ]+[[:space:]]+)*commit([[:space:]]|$)' || exit 0 - +echo "$COMMAND" | grep -qE '(^|[[:space:]])git[[:space:]]+(-c[[:space:]]+[^ ]+[[:space:]]+)*push([[:space:]]|$)' || exit 0 REPO_ROOT=$(git rev-parse --show-toplevel 2>/dev/null) # Pre-push hook: runs tests, then spawns code-reviewer + security-reviewer in # parallel. Blocks the push on (a) test failure, (b) any Critical/Important/ From d7ecef2b345a2405f0f7219707dfe62a530306a4 Mon Sep 17 00:00:00 2001 From: Dominik Schmid Date: Wed, 22 Apr 2026 16:46:37 +0200 Subject: [PATCH 10/10] really the last fix - I know those commit messages are ugly --- .claude/hooks/pre-commit-lint.sh | 1 - .claude/hooks/pre-push-review.sh | 8 ++++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/.claude/hooks/pre-commit-lint.sh b/.claude/hooks/pre-commit-lint.sh index 238bdfe..7544bc1 100755 --- a/.claude/hooks/pre-commit-lint.sh +++ b/.claude/hooks/pre-commit-lint.sh @@ -3,7 +3,6 @@ # against settings.json filter regressions. COMMAND=$(jq -r '.tool_input.command // empty' 2>/dev/null) echo "$COMMAND" | grep -qE '(^|[[:space:]])git[[:space:]]+(-c[[:space:]]+[^ ]+[[:space:]]+)*commit([[:space:]]|$)' || exit 0 -REPO_ROOT=$(git rev-parse --show-toplevel 2>/dev/null) # Pre-commit hook: runs `make lint` in Rust repositories before allowing git commit. # Exit 0 = allow, Exit 2 = block (reason on stderr). diff --git a/.claude/hooks/pre-push-review.sh b/.claude/hooks/pre-push-review.sh index d4f42a3..c3c43c0 100755 --- a/.claude/hooks/pre-push-review.sh +++ b/.claude/hooks/pre-push-review.sh @@ -3,7 +3,6 @@ # against settings.json filter regressions. COMMAND=$(jq -r '.tool_input.command // empty' 2>/dev/null) echo "$COMMAND" | grep -qE '(^|[[:space:]])git[[:space:]]+(-c[[:space:]]+[^ ]+[[:space:]]+)*push([[:space:]]|$)' || exit 0 -REPO_ROOT=$(git rev-parse --show-toplevel 2>/dev/null) # Pre-push hook: runs tests, then spawns code-reviewer + security-reviewer in # parallel. Blocks the push on (a) test failure, (b) any Critical/Important/ # Warning finding from either reviewer, or (c) reviewer crash or malformed @@ -153,7 +152,12 @@ evaluate_reviewer() { if [ "$rc" -ne 0 ]; then echo "${name}: agent exited with status ${rc}; treating as block." >&2 [ -s "$out" ] && cat "$out" >&2 - [ -s "${TMPDIR}/${name_lower}.err" ] && { echo "--- agent stderr ---" >&2; cat "${TMPDIR}/${name_lower}.err" >&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