diff --git a/.claude/commands/tidy-history.md b/.claude/commands/tidy-history.md new file mode 100644 index 00000000..f9521323 --- /dev/null +++ b/.claude/commands/tidy-history.md @@ -0,0 +1,77 @@ +--- +description: "Assess origin/main..HEAD and, if messy, propose then (on approval) perform a commit-history cleanup before or on a pull request" +argument-hint: "[base-ref]" +--- + +Tidy the commit history of the current branch **before it reaches `main`**, per +`CONTRIBUTING.md` ("Branches", "Commit messages") and `AGENTS.md` ("Tidying +history before a pull request"). + +This repository merges pull requests with a **merge commit**: every commit on +the branch lands in `main`'s history, so cleaning it up before merge is not +cosmetic. + +Base to compare against: `origin/main`. If a base ref was passed (`$ARGUMENTS`), +use it in place of `origin/main` everywhere below. + +## 1 — Read the history (change nothing yet) + +Run these and show me the output: + +``` +git fetch origin +git log --oneline --no-merges origin/main..HEAD +git diff --stat origin/main...HEAD +git status --short +``` + +Then confirm the branch is **yours alone**: if anyone else may have based work +on it, STOP — its history MUST NOT be rewritten (CONTRIBUTING.md, "Branches"); +offer only follow-up-commit fixes. + +## 2 — Assess + +Judge every commit in `origin/main..HEAD`. Flag for cleanup: + +- pending autosquash placeholders — `fixup!`, `squash!`, `amend!` (CI rejects them); +- headers that fail the convention — pipe each through the repository's own + linter: `git log -1 --format=%B | tools/commit-lint/lint-commit-message.sh --ci -`; +- a commit that only fixes / rewords / reverts an earlier commit of this same + branch ("wip", "typo", "address review", a commit and its own revert); +- one logical change scattered across commits that don't each stand alone, or two + unrelated intentions folded into one commit. + +If nothing is flagged, tell me the history reads clean and **stop** — do not +rewrite for its own sake. + +## 3 — Propose (rewrite nothing yet) + +If something is flagged, show me a plan as a table — for each commit: +keep / squash-into / reword-to / drop / reorder — plus the resulting +`git log --oneline`, every rewritten header conforming to CONTRIBUTING.md +(`[(scope)][!]: `, +scope from `core, analyzers, binder, cli, dummies, gendoc, testing`). Then **ask +me to approve.** Run no history-rewriting command before I say go. + +## 4 — Rewrite (only after I approve, only while the branch is yours alone) + +- prefer `git rebase --autosquash origin/main` when the cleanup is fixup!/squash! + folding; +- otherwise perform the approved rebase against `origin/main` + (reword / squash / drop / reorder); +- never touch a commit already on `origin/main` — `origin/main..HEAD` is the only + range you may rewrite. + +## 5 — Verify, then publish + +- **The tree must not move.** Prove the cleanup changed only messages and + grouping, never code: `git range-diff origin/main HEAD`, and confirm + `git diff origin/main...HEAD` is identical to before. +- re-lint every resulting commit: + `git log -1 --format=%B | tools/commit-lint/lint-commit-message.sh --ci -`; +- publish with `git push --force-with-lease` — **never** a bare + `git push --force`. + +Report the before/after `git log --oneline` and confirm the diff is unchanged. +Do not open, merge, or enable auto-merge on the pull request — that stays with +the maintainer. diff --git a/.claude/hooks/history-hygiene.sh b/.claude/hooks/history-hygiene.sh new file mode 100755 index 00000000..4704bc17 --- /dev/null +++ b/.claude/hooks/history-hygiene.sh @@ -0,0 +1,141 @@ +#!/bin/sh +# FirstClassErrors — history-hygiene hook. +# +# Wired from .claude/settings.json on two events so an agent never opens a pull +# request on — nor quietly grows — a branch whose commits would land messy in +# `main`. This repository merges pull requests with a merge commit, so every +# commit on a branch reaches protected history (CONTRIBUTING.md, "Branches"): +# a messy history is not squashed away on merge, it stays on `main` for good. +# +# It reuses the repository's single commit linter +# (tools/commit-lint/lint-commit-message.sh), so the hook, the local commit-msg +# hook and the CI gate can never disagree about what a conforming header is. +# +# Modes (argument 1): +# pre-pr PreToolUse on the pull-request-creation tool. BLOCKS (exit 2) +# only when the branch carries CI-fatal history — pending +# fixup!/squash!/amend! placeholders, or headers the linter +# rejects — because that pull request would fail the commit-lint +# CI job and cannot merge as-is. Softer mess (wip-ish commits +# whose headers each lint clean) is left to the agent's judgement +# (AGENTS.md); the hook does not block on taste. +# post-commit PostToolUse on Bash. After a git command that moved the branch +# tip and left origin/main..HEAD messy, prints an advisory +# reminder (exit 2 surfaces it to the agent; nothing is blocked, +# the tool already ran). +# +# The hook never rewrites anything: it only reads and reminds. The judgement of +# "is this messy?" and the decision to rewrite stay with the agent and, for the +# rewrite itself, the maintainer. + +set -u + +mode="${1:-}" + +# Always drain stdin (the harness pipes the hook payload); parsed only in +# post-commit. Draining avoids any broken-pipe noise on the writer side. +payload="$(cat 2>/dev/null || true)" + +root="${CLAUDE_PROJECT_DIR:-$(git rev-parse --show-toplevel 2>/dev/null || true)}" +[ -n "$root" ] || exit 0 # not a repo we understand: stay silent +cd "$root" 2>/dev/null || exit 0 +linter="$root/tools/commit-lint/lint-commit-message.sh" + +# The base we protect. The default branch is `main`; a branch is read against a +# freshly-known origin/main (CONTRIBUTING.md, "Branches"). +base='origin/main' +git rev-parse --verify --quiet "$base" >/dev/null 2>&1 || exit 0 + +range="${base}..HEAD" +commits="$(git rev-list --no-merges "$range" 2>/dev/null || true)" +[ -n "$commits" ] || exit 0 # nothing ahead of main: clean by definition + +# --- classify the range ------------------------------------------------------- +# fatal: would fail the commit-lint CI job (blocks a pull request). +# soft: wip-ish scaffolding; each header may still lint clean (advisory only). +fatal='' +soft='' +for sha in $commits; do + subject="$(git log -1 --format=%s "$sha")" + short="$(git log -1 --format='%h %s' "$sha")" + + case "$subject" in + 'fixup! '*|'squash! '*|'amend! '*) + fatal="${fatal} - ${short} (autosquash placeholder, rejected by CI) +" + continue ;; + esac + + if [ -x "$linter" ] && ! git log -1 --format=%B "$sha" | "$linter" --ci - >/dev/null 2>&1; then + fatal="${fatal} - ${short} (header does not follow CONTRIBUTING.md) +" + continue + fi + + # Soft signals: a subject that reads like scaffolding to squash before merge. + low="$(printf '%s' "$subject" | tr '[:upper:]' '[:lower:]')" + case "$low" in + wip|wip:*|'wip '*|*' wip'|*' wip '*|\ + *typo*|*oops*|*'address review'*|*'review comment'*|*'review feedback'*|\ + *'fix review'*|*'apply review'*|*'self review'*|*'self-review'*|*'pr feedback'*) + soft="${soft} - ${short} +" ;; + esac +done + +fatal_hint() { + cat >&2 <&2 </dev/null 2>&1; then + cmd="$(printf '%s' "$payload" | jq -r '.tool_input.command // empty' 2>/dev/null || true)" + fi + [ -n "$cmd" ] || cmd="$(printf '%s' "$payload" | tr '\n' ' ')" # fallback: raw scan + case "$cmd" in + *'git commit'*|*'git rebase'*|*'git push'*|*'git merge'*|*'git cherry-pick'*|*'git revert'*) : ;; + *) exit 0 ;; # not a history-moving git command + esac + if [ -n "$fatal" ] || [ -n "$soft" ]; then + [ -n "$fatal" ] && fatal_hint + [ -n "$soft" ] && soft_hint + exit 2 # advisory; PostToolUse surfaces it to the agent + fi + exit 0 ;; + + *) + exit 0 ;; +esac diff --git a/.claude/settings.json b/.claude/settings.json new file mode 100644 index 00000000..8be32e1f --- /dev/null +++ b/.claude/settings.json @@ -0,0 +1,26 @@ +{ + "hooks": { + "PreToolUse": [ + { + "matcher": "mcp__github__create_pull_request", + "hooks": [ + { + "type": "command", + "command": "sh \"$CLAUDE_PROJECT_DIR/.claude/hooks/history-hygiene.sh\" pre-pr" + } + ] + } + ], + "PostToolUse": [ + { + "matcher": "Bash", + "hooks": [ + { + "type": "command", + "command": "sh \"$CLAUDE_PROJECT_DIR/.claude/hooks/history-hygiene.sh\" post-commit" + } + ] + } + ] + } +} diff --git a/AGENTS.md b/AGENTS.md index d3289bef..f214bcc2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -50,6 +50,65 @@ merges a pull request. When it is genuinely unclear whether a change is significant enough, or whether it supersedes an existing ADR, say so in the pull request and let `@reefact` judge rather than guessing. +## Tidying history before a pull request (acting agent) + +This governs the agent that *prepares* a branch for review, not the reviewer. +This repository merges pull requests with a **merge commit**, so every commit a +branch carries lands in `main`'s history — a messy branch is not squashed away +on merge, it pollutes protected history for good. `CONTRIBUTING.md` already +fixes the endpoint (autosquash placeholders squashed before merge, a conforming +header on every commit, one intention per commit); this section makes the agent +*reach* it **on its own initiative**, the way it runs the ADR check without +being asked. + +At two moments, read the branch against a freshly fetched `origin/main`: +**before opening a pull request**, and **after pushing further commits to an +already-open one**. + +``` +git fetch origin +git log --oneline origin/main..HEAD +``` + +Judge whether the history reads clean. Treat these as **messy**, worth proposing +a cleanup for: + +- autosquash placeholders still pending — `fixup!`, `squash!`, `amend!` (CI + rejects them); +- a commit that only fixes, rewords, or reverts an earlier commit of the *same* + branch — "wip", "typo", "address review", a commit and its own revert; +- a header that fails the convention — run each through the repository's own + linter, `git log -1 --format=%B | tools/commit-lint/lint-commit-message.sh --ci -`; +- one logical change scattered across commits that do not each stand alone, or + two unrelated intentions folded into one commit (CONTRIBUTING.md, "Commit + messages"). + +When it reads clean, say so in one line and proceed. When it is messy, +**propose** a concrete plan — which commits to squash, reword, drop, or reorder, +and the resulting `git log --oneline` shape — and rewrite only after an explicit +go-ahead. The endpoint is the maintainer's to approve: no agent rewrites a +branch on its own authority any more than it merges one. + +Hard constraints on the rewrite itself (CONTRIBUTING.md, "Branches"): + +- Rewrite history **only while the branch is yours alone**. Once anyone may have + based work on it, a force-push discards that work — leave the history and say + why. +- Publish with `git push --force-with-lease`, never a bare `--force`: the lease + refuses the push if the remote moved under you. +- Never touch a commit already on `main`; `origin/main..HEAD` is the only range + you may rewrite. +- This tidies history, not code. The diff against `origin/main` MUST be identical + before and after — prove it with `git range-diff origin/main HEAD` + (only messages and grouping move, never the tree). + +For Claude Code the mechanics are packaged: the `/tidy-history` command runs the +assessment and, on approval, the rewrite; a hook (`.claude/`, on pull-request +creation and after each committing or pushing git command) flags the CI-fatal +signals so the check is never skipped. Other agents apply the rule by hand. +Either way the judgement — *is this messy?* — and the decision to rewrite stay +here. + ## Review guidelines (pull request reviews) READ THIS BEFORE REVIEWING. The full specification is in `code_review.md`; the diff --git a/CLAUDE.md b/CLAUDE.md index a84bc7f8..e34b8569 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -97,6 +97,7 @@ The essentials, inlined so they hold even if `AGENTS.md` is not read: * Write every commit message per [`CONTRIBUTING.md`](CONTRIBUTING.md): Conventional Commits, a closed type list, the scopes `core, analyzers, binder, cli, dummies, gendoc, testing`, an imperative header within 72 characters, and `Refs: #NN` in a footer when a GitHub issue exists (issue-closing keywords belong in the PR description, not the commit). * Write every pull request title per [`CONTRIBUTING.md`](CONTRIBUTING.md): name the whole change in English; a single-intention PR mirrors its commit header (`type(scope): description`), a multi-intention PR uses a short descriptive title, and issue references stay in the description, not the title. * Enable the local commit-message hook once per clone with `git config core.hooksPath .githooks`; the same check runs in CI on every pull request. +* Before opening a pull request — and after pushing more commits to an open one — read the branch against a fresh `origin/main` and, if the history is messy (pending `fixup!`/`squash!`, wip/typo/"address review" commits, headers the lint rejects, one change split across non-standalone commits or two folded into one), **propose** a cleanup and rewrite only after I approve — while the branch is yours alone, with `git push --force-with-lease`, leaving the diff against `origin/main` unchanged. This repository merges with a merge commit, so a messy branch reaches `main`. Full rule in [`AGENTS.md`](AGENTS.md) ("Tidying history before a pull request"); the `/tidy-history` command runs it. * In PR descriptions, do not invent testing results. Only check items that were actually run. ## Responding to pull request review feedback