diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md new file mode 100644 index 0000000..2aba03c --- /dev/null +++ b/.claude/CLAUDE.md @@ -0,0 +1,57 @@ +# CLAUDE.md + +> Per-project lessons: `.claude/lessons.md`. Read at the start of every non-trivial task. Append new entries with `/codify-lesson `. + +## 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..3e32de2 --- /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/...HEAD -- CHANGELOG.md + ``` + +If either condition is met, output `SKIP: already handled` and stop. + +## Step 2: Analyze the Diff + +Run: +``` +git diff origin/...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: ### Fixes +- Fixed `fetch_notes` pagination race and `:memory:` pool isolation bug ([#77](https://github.com/0xMiden/note-transport-service/pull/77)). +``` + +## 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..0b3f17a --- /dev/null +++ b/.claude/agents/code-reviewer.md @@ -0,0 +1,112 @@ +--- +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`. 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. + +## 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 + +**Findings (Critical, Important) block the merge.** Every issue must be addressed before pushing. + +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 new file mode 100644 index 0000000..ce60377 --- /dev/null +++ b/.claude/agents/security-reviewer.md @@ -0,0 +1,126 @@ +--- +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`. 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. + +## 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 critical or warning findings block the merge.** Every issue must be addressed before pushing. + +**Verdicts:** +- **BLOCK** - Any findings at severity levels Critical or Warning. Do not merge until addressed. +- **CLEAN** - Zero critical or warning 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 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 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 new file mode 100755 index 0000000..4c5676f --- /dev/null +++ b/.claude/hooks/post-pr-create-changelog.sh @@ -0,0 +1,126 @@ +#!/bin/bash +# 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 '(^|[^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 +# 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) + +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') + +[ -z "$PR_URL" ] || [ -z "$PR_NUMBER" ] || [ -z "$CWD" ] && exit 0 + +# ---------------------------------------------------------------------------- +# 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 [ -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 + + 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; } + + 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" + +RESULT_FILE=$(mktemp) +trap 'rm -f "$RESULT_FILE" "$RESULT_FILE.err"' EXIT + +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 + emit_context "No changelog entry needed for this PR. Apply the 'no changelog' label now: + +gh pr edit ${PR_NUMBER} --add-label 'no changelog'" + exit 2 +fi + +if [[ "$VERDICT" == CHANGELOG:* ]]; then + ENTRY=$(sed -n '/^CHANGELOG:/,$ { s/^CHANGELOG: //; p }' "$RESULT_FILE") + + if VERSION=$(cd "$CWD" && resolve_unreleased_version "$PR_NUMBER"); then + VERSION_INSTRUCTION="Add the following to CHANGELOG.md under the ${VERSION} unreleased section (resolved from milestone)" + else + VERSION_INSTRUCTION="Add the following to CHANGELOG.md under the appropriate unreleased section. WARNING: I could not resolve the target version from the PR milestone or any open milestone - ask the user which version to file under before committing" + fi + + emit_context "Changelog entry needed for PR #${PR_NUMBER}. ${VERSION_INSTRUCTION}, then commit and push: + +${ENTRY}" + exit 2 +fi + +# 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 new file mode 100755 index 0000000..7544bc1 --- /dev/null +++ b/.claude/hooks/pre-commit-lint.sh @@ -0,0 +1,34 @@ +#!/bin/bash +# 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 + +# 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 100755 index 0000000..e874116 --- /dev/null +++ b/.claude/hooks/pre-pr-draft.sh @@ -0,0 +1,80 @@ +#!/bin/bash +# 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 '(^|[^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 +# `gh pr ready ` 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=$(printf '%s' "$INPUT" | jq -r '.tool_input.command // empty' 2>/dev/null || true) + +if [ -z "$COMMAND" ]; then + exit 0 +fi + +# 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 + +# 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 new file mode 100755 index 0000000..c3c43c0 --- /dev/null +++ b/.claude/hooks/pre-push-review.sh @@ -0,0 +1,196 @@ +#!/bin/bash +# 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:]]+)*push([[:space:]]|$)' || exit 0 +# 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 + +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 "Pre-push: cannot determine diff base; falling back to HEAD~1." >&2 + BASE="HEAD~1" +fi + +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 + +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" + +echo "Pre-push: spawning code-reviewer + security-reviewer..." >&2 + +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> "$TMPDIR/sec.err" & +PID_SEC=$! + +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 + + 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 + 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 +} + +BLOCKED=0 +evaluate_reviewer "CODE REVIEWER" "$RC_CODE" "$CODE_OUT" || BLOCKED=1 +evaluate_reviewer "SECURITY REVIEWER" "$RC_SEC" "$SEC_OUT" || BLOCKED=1 + +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 + +echo "" >&2 +echo "Pre-push: all checks passed." >&2 +exit 0 \ No newline at end of file diff --git a/.claude/lessons.md b/.claude/lessons.md new file mode 100644 index 0000000..bf7271d --- /dev/null +++ b/.claude/lessons.md @@ -0,0 +1,43 @@ +# Project Lessons - note-transport-service + +> 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/.claude/settings.json b/.claude/settings.json new file mode 100644 index 0000000..e2aa027 --- /dev/null +++ b/.claude/settings.json @@ -0,0 +1,38 @@ +{ + "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" + }, + { + "type": "command", + "if": "Bash(*gh pr create*)", + "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 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