Add a pre-push check suite

.claude/skills/check-docs/SKILL.md

@@ -0,0 +1,39 @@
+---
+name: check-docs
+description: Check that a branch leaves the docs true, necessary and short, including comments and docstrings. Use this during every code review.
+---
+
+Review the prose the diff adds or leaves behind: README.md, AGENTS.md, code comments and docstrings.
+
+Where facts live:
+
+- README.md is for someone using diffui: install, CLI flags, keybindings, state files, development setup.
+- AGENTS.md is for someone changing it: architecture, patterns, gotchas that break silently, commands.
+- A comment or docstring is for a fact tied to one function, and it must explain what the code can't show.
+
+## What to check
+
+Ask these in order. Check 1 blocks. The rest are suggestions, except a banned tell in check 5, which blocks.
+
+1. Does the diff make an existing doc claim false? Search README.md and AGENTS.md for the names, paths, flags, keybindings, state-file fields and counts (themes, scopes, routes) the diff changes. Check the architecture tree in AGENTS.md for added, moved or deleted files, and the Testing section for added or removed test files. A claim the diff breaks must be fixed or deleted in the same branch.
+2. Did the diff change a decision a doc explains? If a page gives the reason for a design choice and the branch changes the choice, update the reason. Changed mechanics under an unchanged decision need nothing.
+3. Does the added doc text belong? Flag file maps, route or flag tables the code already shows, function-by-function walks, and changelog wording ("now", "no longer", "previously"). Never flag a missing doc just because code changed.
+4. Does each added comment or docstring earn its place? Grep the nearest sibling modules first: diffui comments are sparse and explain a quirk, a git edge case or a reason a reader would "fix" the code wrongly. Flag comments that restate the line below, docstrings for code that doesn't exist yet, and meta text that reads fine with the code deleted (why it was added, what it replaced, who asked).
+5. Does added prose sound like Dillon and avoid the AI tells? Read `~/.claude/STYLE.md` and `~/.claude/TROPES.md` if you can. American English, plain, practical, leads with what changed or what to do. Flag em-dashes, "rather than" and "instead of", negative parallelism ("not X, it's Y"), bold-first bullets, "lands in/on" as a placement verb, and the banned vocabulary in TROPES.md (leverage, robust, seamless, streamline and the rest).
+6. Is the added text as short as it can be? Flag a point made twice, hedging, a sentence carrying two ideas, and a lead buried under setup. A compressed sentence that takes longer to decode than the code is worse than none.
+
+AGENTS.md loads on every turn, so a paragraph added to it should be something an agent needs while working. A paragraph that only describes how a subsystem works belongs in README.md or a docstring, and one that restates another doc should be cut.
+
+## How to flag
+
+> :warning: Stale doc: `README.md` says [claim], which this branch changes to [new behavior]. Fix or delete that sentence here.
+
+> :warning: Writing tell: `path:42` uses [em-dash / "rather than" / bold-first bullet / ...]. Rewrite: "...".
+
+> :scissors: Doc bloat: the lines added to `AGENTS.md` [describe]. The code shows this, and it goes stale on the next change. Cut it or move it to [place]. Suggestion, not a blocker.
+
+> :pencil2: Doc clarity: `path:42` [restates the code / is meta / says X twice / buries the point]. [Cut it. / Suggested rewrite: "..."] Suggestion, not a blocker.
+
+When there is nothing to flag, report only this line:
+
+> ✅ Docs (check-docs): no changes needed.

.claude/skills/check-scope/SKILL.md

@@ -0,0 +1,37 @@
+---
+name: check-scope
+description: Check that a branch's changes trace to its stated purpose, contain nothing speculative, and are small enough to review safely. Use this during every code review.
+---
+
+Judge whether every change serves the stated purpose, and whether the branch is too big to review safely. Analyze and suggest only. Never split, branch, cherry-pick or edit.
+
+The stated purpose is the commit messages on the branch, given in the prompt. When the messages are too thin to judge from, say so and judge on coherence alone.
+
+## What to check
+
+Ask these in order. Checks 1 and 2 block. The rest are suggestions.
+
+1. Does a change fail to trace to the stated purpose? Flag drive-by fixes, reformatting or comment edits in lines the purpose doesn't touch, renames of adjacent code, and dependency bumps. Match the style of the surrounding code even where another style would be better. Name the hunk and the purpose it doesn't serve.
+2. Is anything speculative? Flag helpers with no caller, config knobs nobody sets, retries, fallbacks, caches and guards for states the callers can't produce, abstractions for a second case that doesn't exist, and docstrings for code that isn't there. A not-yet-called module is fine only when it is the branch's stated deliverable and names its consumer. "To be safe" is not a requirement.
+3. Did the change leave dead code behind? Code, imports, tests or docs the branch made obsolete should go in the same diff.
+4. Does it add structure the request didn't imply? A new file, class or dependency when an edit to an existing file would do, a bundler or build step for the frontend (diffui has none and must not gain one), or a wrapper around a single call. Inline a helper until it has three real callers.
+5. Is it bigger than the problem? Flag 200 lines that could be 50, and an abstraction for two call sites that inlining would shorten.
+6. Is it too large to review? Count substantive lines, excluding tests, docs, renames and formatter output. Over about 15 files or 800 substantive lines, or more than one unrelated concern at any size, look for a split. Concerns matter more than line count: one refactor plus its one consumer is a good branch, and a small one mixing a bug fix, a UI tweak and a dependency bump is not.
+
+## Splits
+
+Cut only along real seams: pure refactor before behavior change, new module before wiring, drive-by fixes lifted out, dead-code removal. Never cut by line count, file count or directory. Each part must pass the suite alone, revert alone, deliver something real and carry its own tests. Suggest at most three parts. If no seam survives, report the branch as it is, since a bad split costs the author time to reject.
+
+## How to flag
+
+> :warning: Out of scope: `path:42` [what changed], which doesn't serve [purpose]. Drop it or move it to its own branch.
+
+> :warning: Speculative: `path:42` [helper / knob / fallback / guard] has [no caller / no setter / no reachable trigger]. Delete it, or name the requirement that needs it.
+
+> :mag: Simplification: `path:42` [what is unnecessary and what to do instead]. Removes about [N] lines. Suggestion, not a blocker.
+
+> :scissors: Split suggestion: this branch changes [N] files and about [M] substantive lines across [concerns]. Consider [Part 1: files, what it delivers alone] then [Part 2: files, depends on Part 1]. Suggestion, not a blocker.
+
+When there is nothing to flag, report only this line:
+
+> ✅ Scope (check-scope): no changes needed.

.claude/skills/check-tests/SKILL.md

@@ -0,0 +1,42 @@
+---
+name: check-tests
+description: Check that a branch covers the behavior it changes, and that every test it adds earns its pipeline time. Use this during every code review.
+---
+
+Review the behavior the non-test code changes and every test the diff adds or edits. Read each changed test next to the code it calls. Open an unchanged test only to confirm a redundant finding. Don't run the tests.
+
+A test earns its place by catching a named bug that no existing test catches. If you can't name the bug, it is a finding. Tests live in `tests/`: `test_server.py` drives the routes through a `TestClient`, `test_tui.py` drives the real `DiffuiApp` under `run_test()`, and the rest cover one module each.
+
+## What to check
+
+Ask these in order. The first two block. The rest are suggestions.
+
+1. Is a behavior change untested? For each behavior the diff adds or fixes, find the test that fails without it. A new route, branch or error path nothing drives is a gap. A bug fix needs its regression test. Don't ask for tests of code the branch didn't change.
+2. Can each test fail? Flag a test that passes however the code behaves: a body that never asserts, a status check that accepts a 500, an `or` clause that is always true, an assertion on only `is not None`, `isinstance` or `len(x) > 0`, a check against a fixture that never held the thing it asserts is absent, an ordering check against a fake that sorts its own rows. If deleting the line under test or stubbing the function to return None leaves it green, it can't fail.
+3. Is it tied to the implementation? Flag assertions on mock call counts, `call_args`, substrings of generated git arguments or HTML, and private state. Assert what a caller sees: a return value, a response body, a file written, an error raised.
+4. Does it test a library? Pydantic validation and defaults, FastAPI's 422s, Textual's own behavior, Pygments tokenizing and `pty` mechanics aren't ours to test.
+5. Is it redundant? Name the test that already drives the same path (`file:line`) or it isn't a finding. Prefer a parametrize case on the existing test over a new test or file.
+6. Is it the wrong kind? Default to a feature test that exercises the code the way its caller does: a real request against `temp_repo`, or the real app under `run_test()`. Keep unit tests for pure, intricate logic such as diff parsing and line numbering. Flag a unit test that mocks away the thing a feature test would cover cheaply.
+7. Is it slow or flaky? A real `sleep` where the `until()` helper exists, a fresh server per test where a session fixture would do, or a wait on a fixed delay.
+
+When one change needs many tests, question the code before the tests: a helper whose cache and fallback paths need six tests is usually a helper that shouldn't cache or fall back. When a change makes a test obsolete or a new test subsumes an old one, the old test should be deleted in the same diff.
+
+Judge tests of path containment and anything that writes to the working tree on duplication only, however simple they look. Don't flag length or style. When several tests share a fix, list them on one finding.
+
+## How to flag
+
+> :warning: Untested change: `path/module.py:42` [behavior], and no test fails without it. Add a [feature] test that [does X and asserts Y].
+
+> :warning: Can't fail: `tests/test_x.py:88` `test_name` [why it passes regardless]. Fix the assertion or delete the test.
+
+> :scissors: Test to cut: `tests/test_x.py:120` `test_name` [repeats `tests/test_y.py:40` / tests Pydantic's required-field check]. Delete it or fold it into the parametrize at `tests/test_x.py:100`. Suggestion, not a blocker.
+
+> :link: Tied to the implementation: `tests/test_x.py:60` `test_name` asserts [a mock call count / a substring / private state]. Assert [the response / the file written] instead. Suggestion, not a blocker.
+
+> :turtle: Slow or flaky: `tests/test_x.py:30` [sleep / per-test server]. Use [`until()` / the session fixture]. Suggestion, not a blocker.
+
+When there is nothing to flag, report only this line:
+
+> ✅ Tests (check-tests): no changes needed.
+
+Suggest, never edit. The author makes the fix.

.githooks/pre-push

@@ -0,0 +1,28 @@
+#!/usr/bin/env bash
+# Enable once per clone: git config core.hooksPath .githooks
+set -euo pipefail
+
+cd "$(git rev-parse --show-toplevel)"
+
+# Git sends the pushed refs on stdin. Read them before the checks below can consume them.
+refs=$(cat)
+
+echo "Running pre-push checks..."
+
+echo "  ruff check..."
+ruff check .
+
+echo "  ruff format --check..."
+ruff format --check .
+
+echo "  pytest..."
+pytest -q
+
+if [[ ",${SKIP:-}," == *",check-suite,"* ]]; then
+    echo "  check-suite skipped."
+else
+    echo "  check-suite..."
+    printf '%s\n' "$refs" | python3 scripts/check_suite.py
+fi
+
+echo "All checks passed."

AGENTS.md

@@ -149,16 +149,20 @@ diffui --json                        # Export the review session as JSON
 pytest                               # Run tests
 ruff check .                         # Lint
 ruff format --check .                # Format check
+git config core.hooksPath .githooks  # Once per clone: enables the pre-push hook
 ```
 
 CI (`.github/workflows/ci.yml`) runs `ruff check`, `ruff format --check` and `pytest` on Ubuntu and macOS, Python 3.10 and 3.13. The checkout is full-depth because some tests diff the repo against `origin/main`.
 
+`.githooks/pre-push` runs the same three checks, then `scripts/check_suite.py` (the README's Development section describes it). Change what the review looks for in the skills, not in the script.
+
 ## Testing
 
 - `tests/conftest.py` points `_CONFIG_ROOT` at a temp dir for the whole session, clears `HERDR_WORKSPACE_ID`, and builds the `temp_repo` fixture (a `feature` branch with a dirty tree and an untracked file). Server tests use a `TestClient` with `base_url="http://localhost"`.
 - `tests/test_server.py` covers the routes. Its `fake_herdr` fixture puts a fake `herdr` on `PATH`.
 - `tests/test_tui.py` runs the real `DiffuiApp` under `App.run_test()` against a `LocalServer` on `temp_repo`. Tests drive the app with `asyncio.run` and wait with `until()`, because the server reaches the view through the watcher and SSE.
 - `test_diff.py`, `test_highlight.py`, `test_git_utils.py`, `test_events.py`, `test_agent_terminal.py` and `test_themes.py` cover the matching modules.
+- `test_check_suite.py` covers the verdict parsing and skip paths of `scripts/check_suite.py` with `subprocess` patched. It never starts `claude`.
 
 ## State files
 

README.md

@@ -253,11 +253,14 @@ Three things live elsewhere. The last-turn baseline is a git ref in the repo's o
 
 ```bash
 pip install -e ".[dev]"
+git config core.hooksPath .githooks   # once per clone
 pytest
 ruff check .
 ruff format --check .
 ```
 
+`.githooks/pre-push` runs ruff and pytest, then `scripts/check_suite.py`. That script asks a headless `claude` session to review the branch against `origin/main` with the skills in `.claude/skills/` (check-docs, check-scope, check-tests) and blocks the push on a `:warning:` finding. It skips itself when `claude` isn't installed or gives no verdict. `SKIP=check-suite git push` skips it on purpose. The hooks path is relative, so one `git config` covers every worktree of the clone.
+
 AGENTS.md has the architecture.
 
 ## License

scripts/check_suite.py

@@ -0,0 +1,104 @@
+"""Run the review check skills on a branch before it is pushed.
+
+One headless Claude session runs check-docs, check-scope and check-tests against
+origin/main and blocks the push when any of them raises a :warning: finding.
+The pre-push hook pipes git's `<local ref> <local sha> <remote ref> <remote sha>`
+lines to stdin.
+"""
+
+from __future__ import annotations
+
+import re
+import shutil
+import subprocess
+import sys
+
+SKILLS = ("check-docs", "check-scope", "check-tests")
+TIMEOUT_SECONDS = 600
+VERDICT = re.compile(r"CHECK-SUITE:\W*(PASS|BLOCK)\b")
+PROMPT = """Review this branch as a code review. Read each of these skill files \
+and follow it: {skills}. Report each one's output in its own format. You can't \
+run shell commands, so use the diff below wherever a skill asks for `git diff`, \
+and read files directly. Do not edit any files. End with exactly one line: \
+`CHECK-SUITE: BLOCK` if any skill raised a :warning: finding, otherwise \
+`CHECK-SUITE: PASS`.
+
+Commit messages on the branch, which state its purpose:
+
+{log}
+
+`git diff --stat -M {range}`:
+
+{stat}
+
+`git diff -M {range}`:
+
+{diff}"""
+
+
+def pushed_sha(hook_input: str) -> str | None:
+    """Return the first local sha being pushed, skipping branch deletions."""
+    for line in hook_input.splitlines():
+        fields = line.split()
+        if len(fields) == 4 and fields[1].strip("0"):
+            return fields[1]
+    return None
+
+
+def git(*args: str) -> subprocess.CompletedProcess[str]:
+    return subprocess.run(["git", *args], capture_output=True, text=True)
+
+
+def main() -> int:
+    if shutil.which("claude") is None:
+        print("check-suite: claude is not installed, skipping.")
+        return 0
+    hook_input = "" if sys.stdin.isatty() else sys.stdin.read()
+    sha = pushed_sha(hook_input)
+    if hook_input.strip() and sha is None:
+        print("check-suite: the push only deletes refs, skipping.")
+        return 0
+    tip = sha or "HEAD"
+    diff_range = f"origin/main...{tip}"
+    diff = git("diff", "-M", diff_range)
+    stat = git("diff", "--stat", "-M", diff_range)
+    if diff.returncode != 0:
+        print(f"check-suite: git diff {diff_range} failed, skipping.\n{diff.stderr}")
+        return 0
+    if not diff.stdout:
+        return 0
+    log = git("log", "--format=%B", f"origin/main..{tip}")
+
+    print(f"check-suite: running {', '.join(SKILLS)} on {diff_range}. This takes a few minutes.")
+    skill_files = ", ".join(f".claude/skills/{skill}/SKILL.md" for skill in SKILLS)
+    prompt = PROMPT.format(skills=skill_files, range=diff_range, log=log.stdout, stat=stat.stdout, diff=diff.stdout)
+    try:
+        result = subprocess.run(
+            ["claude", "-p", "--restricted", "--tools", "Read,Grep,Glob", "--strict-mcp-config"],
+            input=prompt,
+            capture_output=True,
+            text=True,
+            timeout=TIMEOUT_SECONDS,
+        )
+    except subprocess.TimeoutExpired:
+        print(f"check-suite: no result after {TIMEOUT_SECONDS}s, skipping.")
+        return 0
+    print(result.stdout)
+    if result.returncode != 0:
+        print(result.stderr, file=sys.stderr)
+        print(f"check-suite: claude exited {result.returncode}, skipping.")
+        return 0
+
+    verdicts = VERDICT.findall(result.stdout)
+    if not verdicts:
+        print("check-suite: no CHECK-SUITE verdict in the report, skipping.")
+        return 0
+    # The prompt quotes a verdict line, so the report can echo one before its own.
+    if verdicts[-1] == "BLOCK":
+        print("check-suite: fix the :warning: findings above, commit, and push again.")
+        return 1
+    return 0
+
+
+if __name__ == "__main__":
+    sys.exit(main())

tests/conftest.py

@@ -27,6 +27,9 @@ def diffui_config_root(tmp_path_factory):
     mp.setattr(diffui.git_utils, "_CONFIG_ROOT", root)
     # A suite run from a herdr pane would otherwise write to the live sidebar.
     mp.delenv("HERDR_WORKSPACE_ID", raising=False)
+    # A pre-push hook in a linked worktree exports GIT_DIR, which points the temp repos' git calls at the checkout.
+    for var in ("GIT_DIR", "GIT_WORK_TREE", "GIT_INDEX_FILE"):
+        mp.delenv(var, raising=False)
     yield root
     mp.undo()