review-commits

name	review-commits
description	Review and clean up commits before pushing. Uses planner/executor split: reads commits inline, authors final messages in saved style, replays via `git rebase --exec` with GIT_EDITOR=true so no editor opens. Detects logical clusters of granular TDD commits and proposes collapsing them into one logical commit. Auto-amends drifted subjects on revalidation. Activates on: "review commits", "clean up commits", "prep for push", "squash these commits", "finalize commits", "merge branch to main", "clean up my commits before pushing", "review before push".
argument-hint	[--tag <version>] [--base <ref>]

Review Commits

Review and clean up commits before pushing. Auto-detects whether you are on a feature branch or on main and runs the appropriate workflow. Uses a planner/executor split: all commit message authoring happens in the planning phase, and the executor is purely mechanical replay via git rebase --exec with GIT_EDITOR=true so no editor ever opens mid-rebase. A revalidation step auto-amends any subject that drifts from the saved style.

Arguments

Argument	Type	Default	Description
--tag <version>	string	none	Tag after completion
--base <ref>	string	auto	Override base commit detection

Precondition

Require a clean worktree. Run:

git status --porcelain

If output is non-empty, refuse to proceed:

Your worktree has uncommitted changes. Please commit or stash them
before running review-commits.

Auto-Detection

Check the current branch:

branch=$(git branch --show-current)

• If $branch is main or master → Main Flow
• Otherwise → Branch Flow

Test Detection (Shared)

Before either flow, detect the project's test command by checking for project files in the working directory root:

File	Command
go.mod	go test ./...
package.json	npm test
Cargo.toml	cargo test
pyproject.toml	pytest

Check files in this order. Use the first match. If no project file is found, skip testing and warn: "No test command detected. Skipping tests."

Codebase Index Resolution (Shared)

Before either flow, attempt to set up codebase-memory-mcp for semantic commit analysis. This enables Layer 3 of the hygiene analysis.

Step 1: Check MCP Availability

Call the codebase-memory-mcp list_projects tool.

• If the MCP server is not available (tool call fails or server not connected): set $SEMANTIC_AVAILABLE=false. Continue without semantic analysis — Layers 1 and 2 will still run.
• If the MCP server is available: continue to Step 2.

Step 2: Decide Whether to (Re)Index

Compute three signals:

last_indexed=$(awk '/^last_indexed:/{print $2}' .claude/codebase.local.md 2>/dev/null)
latest_commit_ts=$(git log -1 --format=%cI "$base..HEAD" 2>/dev/null)
needs_reindex=false

if [[ -z "$last_indexed" ]]; then
  needs_reindex=true                          # first run on this project
elif [[ -n "$latest_commit_ts" && "$latest_commit_ts" > "$last_indexed" ]]; then
  needs_reindex=true                          # commits being reviewed are newer than the graph
else
  # 24-hour calendar fallback for the rest of the repo
  now_s=$(date +%s)
  idx_s=$(date -j -f "%Y-%m-%dT%H:%M:%SZ" "$last_indexed" +%s 2>/dev/null || echo 0)
  age_h=$(( (now_s - idx_s) / 3600 ))
  [[ $age_h -gt 24 ]] && needs_reindex=true
fi

If $needs_reindex=true, invoke /codebase:index. The codebase plugin honors the user's auto_index: ask|always|never preference, so the skill delegates that decision rather than re-implementing it.

If indexing fails for any reason, set $SEMANTIC_AVAILABLE=false and continue — do not block the workflow.

If indexing succeeds (or was unnecessary), set $SEMANTIC_AVAILABLE=true.

Argument Parsing

Parse --tag <version> and --base <ref> only. Both are optional. If .claude/clean-merge.local.md exists from a previous v1 run, ignore it — v2 has no settings.

Commit Hygiene Analysis (Shared)

Analyze commits in the range $base..HEAD for hygiene issues. This analysis runs in both Branch Flow and Main Flow (Rebase option) before the rebase plan is built. It is performed inline by the main agent using the checklist in lib/synthesizer-prompt.md.

Three detection layers run in order. Each layer adds findings to a shared list.

Layer 1: Subject-Based (always runs)

Parse git log --oneline $base..HEAD and detect introduce-then-fix pairs:

A pair is flagged when:

• Commit B's subject starts with Fix, Fixup, Fix up, Correct, or Update (case-insensitive)
• The remainder of B's subject shares a significant word (3+ characters, excluding stop words: the, for, and, a, an, in, of, to, is, it, on, at, by, or, be, as, do, if, so, no, up, we, my, he) with commit A's subject
• B is within 3 commits of A in the log (not necessarily adjacent)

Each detected pair is recorded as:

finding: pair
commits: [<hash-A>, <hash-B>]
reason: "introduce-then-fix: <shared word>"
action: fixup (mandatory)

Layer 2: File-Structural (always runs)

For each commit in range, run:

git diff-tree --no-commit-id --name-only -r <hash>

Group changed files by top-level directory. Flag a commit as non-atomic if it touches files in 2+ top-level directories that serve different concerns. Use these heuristics:

• plugins/<name-A>/ and plugins/<name-B>/ are different concerns (unless both are in the same plugin)
• docs/ changes alongside plugins/ are acceptable (docs for the plugin)
• tests/ alongside src/ for the same module is acceptable
• Root config files (.eslintrc, package.json) alongside source changes are acceptable if the config change supports the source change

Each flagged commit is recorded as:

finding: non-atomic
commit: <hash>
concerns: ["<dir-A>: <description>", "<dir-B>: <description>"]
action: edit (suggested)

Test-with-impl detection. If commit B touches only test files (*.test.*, tests/, *_test.*, *_spec.*) for symbols introduced in an earlier commit A, mark B as a fixup of A:

finding: pair
commits: [<hash-A>, <hash-B>]
reason: "test-with-impl: tests for A's symbols arrived in B"
action: fixup (mandatory)

The synthesizer is the right phase to run this check — it has the full file list per commit and can match test files to product files by base name.

Also check if any commit's changes are unrelated to the branch's overall purpose. Compare each commit's changed files against the full branch diff (git diff --name-only $base..HEAD). If a commit only touches files that no other commit in the range touches AND those files are in a different area from the branch's main work, flag it:

finding: unrelated
commit: <hash>
reason: "<description of why it seems unrelated>"
action: drop (suggested)

Layer 3: Semantic (runs when $SEMANTIC_AVAILABLE=true)

For each commit flagged as potentially non-atomic by Layer 2, and for any commit where Layer 2 was uncertain, use codebase-memory-mcp to verify:

1. Get changed symbols: For each changed file in the commit, call search_graph with the file path to find the symbols it defines.

2. Check cluster membership: Call search_graph with a semantic query describing each changed symbol. If changed symbols belong to different clusters (e.g., "AuthService" vs "Observability"), the commit is non-atomic.

3. Trace connections: Call trace_path between changed files. If a path exists (they're connected through call chains), the changes may be related despite being in different directories. Remove the non-atomic flag.

4. Check module boundaries: Call get_architecture and verify whether the commit crosses module boundaries. Cross-boundary changes with no call-chain connection are non-atomic.

Layer 3 can upgrade a Layer 2 finding (uncertain → confirmed non-atomic) or dismiss it (files are in different directories but semantically connected).

If any codebase-memory-mcp tool call fails during Layer 3, skip the remaining semantic checks for that commit and keep the Layer 2 finding as-is. Do not fail the workflow.

Present Hygiene Findings

After all layers complete, present findings using AskUserQuestion if any issues were found:

Commit hygiene analysis complete.

⚠ Issues detected:

  Introduce-then-fix pairs (will be auto-squashed):
    c844c6a + 7de838b — "sync-beads": add then fix shell escaping
    c8fa279 + e526d22 — "hook": expand then fix false-positive

  Non-atomic commits (suggest splitting):
    a1b2c3d — touches auth handler (AuthService) + logging (Observability)
      These are unrelated concerns in different subsystems.

  Unrelated changes (suggest dropping):
    f4e5d6c — README typo fix, unrelated to auth migration branch

Introduce-then-fix pairs are mandatory fixups. For other findings:

○ Accept all suggestions
○ Review one by one — decide per finding
○ Dismiss all — keep commits as-is (only hygiene pairs will be fixed)

If "Review one by one": walk through each non-mandatory finding with AskUserQuestion offering accept/dismiss per finding.

If no issues detected, display:

Commit hygiene analysis: all commits are clean and atomic. ✓

And proceed directly to the rebase plan.

Logical Clustering (Shared)

Beyond per-commit hygiene, the planner detects "feature clusters": runs of 4+ contiguous commits that together form a single logical change (TDD discipline produces these — scaffold → helper → test → another helper → wire it up). Once the feature lands, that granularity is noise on main.

Detection runs after Hygiene Layers 1-3 and uses lib/detect-clusters.sh "$base":

• The helper outputs one line per cluster, space-separated short-hashes oldest-first
• A cluster is 4+ contiguous commits whose touched files all share a 3-level path prefix (e.g. plugins/foo/skills/bar/)
• Commits that are already a fixup target from Layer 1 are excluded from cluster candidates (they collapse separately)

If $SEMANTIC_AVAILABLE=true, the planner confirms each candidate via codebase-memory: query search_graph for the symbols across all commits in the cluster, and reject the candidate if symbols span 2+ unrelated architectural clusters. Confirmation upgrades the proposal to high-confidence; absence of MCP demotes it to medium-confidence (still proposed, the plan review just flags the confidence level).

A confirmed cluster maps to existing plan primitives:

• first commit: action: pick with new_message (authored in saved style)
• every later commit in the cluster: action: fixup

build-todo.sh already emits the right exec sequence for this combination.

Branch Flow

Step 1: Verify Tests

Run the detected test command. If it fails, stop:

Tests failed. Fix the failures before reviewing commits.

Step 2: Show Branch State

Determine the merge base:

base=$(git merge-base HEAD main)

If --base was provided, use that instead.

Show the commits:

git log --oneline $base..HEAD

Display:

Branch <branch-name> has N commits since main:

  <hash> <subject>
  <hash> <subject>
  ...

Analyzing commit hygiene and building rebase plan...

Step 3: Ensure Codebase Index

Run the "Codebase Index Resolution" flow defined above. This sets $SEMANTIC_AVAILABLE for use in Step 4.

Step 4: Plan (inline)

Set up the working directory:

WORKING_DIR=$(mktemp -d)/review-commits
mkdir -p "$WORKING_DIR/msgs"

Resolve $STYLE_FILE by reading .claude/git-commit.local.md → commit_style value → plugins/commit-tools/styles/<style>.md. If the local settings file is missing, default to classic.

Read each commit in $base..HEAD oldest-to-newest using git log --reverse --format='%H%x00%s%x00%b%x00---END---' "$base..HEAD" so commit bodies are available to the synthesizer. Run the planning checklist at plugins/commit-tools/skills/review-commits/lib/synthesizer-prompt.md and write $WORKING_DIR/plan.yaml. Cluster detection runs as part of that checklist (Step 4 in the synthesizer prompt) using lib/detect-clusters.sh.

There is no longer a multi-agent path — the main agent does both reading and synthesis. For a typical feature branch (5-30 commits) this is faster and cheaper than dispatching subagents.

Step 5: Plan Review (User Gate)

Show the plan via AskUserQuestion. Display the original git log --oneline $base..HEAD, then the proposed action sequence with the first line of each new message inline (truncated to 72 chars).

First, present any cluster proposals. The form depends on which detectors fired:

Both detectors fire (overlap-free disjoint case):

Cluster proposals for branch <name>:

  Option A (recommended, branch heuristic, medium confidence):
    Collapse all 4 commits into one
      pick   f3696d7  Extract netbox process-compose commands to scripts
      fixup  bc7a342  ↳ folded
      fixup  fd11711  ↳ folded
      fixup  c0cc5e8  ↳ folded

  Option B (path heuristic, high confidence):
    Collapse 4 commits in plugins/foo/skills/bar/ into one
      pick   9e14e75  Add NetBoxClient interface, mock, and validation
      fixup  3312c0b  ↳ folded
      fixup  …
      fixup  …

○ Accept Option A — collapse the whole branch
○ Accept Option B — collapse only the sub-clusters
○ Keep all as pick — don't collapse
○ Modify per-commit

Branch detector only (no path sub-clusters):

Cluster proposal for branch <name>:

  Branch heuristic, medium confidence:
    Collapse all N commits into one
      pick   <h1>  <subject>
      fixup  <h2>  ↳ folded
      ...

  No path-prefix sub-clusters found.

○ Accept — collapse the whole branch
○ Keep all as pick — don't collapse
○ Modify per-commit

Path detector only (running on main/master, or branch candidate did not fire):

Cluster proposals (path heuristic, high confidence):
  Collapse N commits in <dir> into one
    pick   <h1>  <subject>
    fixup  <h2>  ↳ folded
    ...

○ Accept — collapse the sub-cluster(s)
○ Keep all as pick — don't collapse
○ Modify per-commit

Neither detector fires:

No cluster proposals.
  Branch heuristic: skipped (on main/master)
  Path heuristic: no contiguous run of 4+ commits shares a parent directory.

  If these commits form one logical feature, use Modify to choose
  which to collapse manually.

After the cluster decision is made, present the final per-commit plan:

Rebase plan for N commits on <branch-name>:

  Current log:
    c844c6a  Add sync-beads skill
    7de838b  Fix sync-beads skill
    ...

  Proposed sequence:
    pick   9e14e75  Add NetBoxClient interface, mock, and validation
                      Adds NetBoxClient as the testable boundary for all
                      NetBox HTTP calls. The mock is generated via…
    fixup  3312c0b  ↳ folded into above
    edit   a1b2c3d  Update auth handler  ← will split into 2
    drop   f4e5d6c  Fix unrelated typo

○ Accept — execute this plan
○ Modify — adjust action for specific commits
○ Reset — soft-reset and regroup manually (escape hatch)

For any pick or reword whose authored new_message carries a body, show the first 2 wrapped lines of the body indented under the subject (max ~160 chars total, ellipsis if truncated). Subject-only entries render as today. fixup, drop, and edit entries never get a body preview line — only pick and reword carry new_message.

If "Modify": use AskUserQuestion to let the user change the action for any commit. Re-display the updated plan for confirmation.

If "Reset": fall back to the Soft-Reset Escape Hatch (see below).

Step 6: Execute

Build the todo file and message directory, then run the rebase:

msgdir="$WORKING_DIR/msgs"
mkdir -p "$msgdir"
bash plugins/commit-tools/skills/review-commits/lib/build-todo.sh \
  "$WORKING_DIR/plan.yaml" "$msgdir" > "$WORKING_DIR/todo"
GIT_EDITOR=true \
GIT_SEQUENCE_EDITOR="cat $WORKING_DIR/todo >" \
  git rebase -i "$base"

The executor authors NO messages. If you find yourself writing a commit message during this step, you are violating the contract — stop, return to Step 4, and fix the plan instead.

build-todo.sh and apply-split.sh set __GIT_COMMIT_PLUGIN__=1 on the git commit --amend and git commit -F calls they emit/run, which is the documented bypass for the git-commit plugin's PreToolUse block. The skill itself does not need to re-set this — the helpers handle it.

If the rebase reports a conflict:

1. Stop immediately
2. Run git diff --name-only --diff-filter=U to show conflicting files
3. Await user resolution + git rebase --continue
4. Resume from Step 7 once the rebase completes

Step 7: Revalidate

PATH_TO_LIB=plugins/commit-tools/skills/review-commits/lib \
  bash plugins/commit-tools/skills/review-commits/lib/revalidate.sh \
  "$WORKING_DIR/plan.yaml" "$msgdir" "$base" "$STYLE_FILE" \
  || echo "drift remained — see warnings"

This auto-amends any commit whose subject drifted from the saved messages. A non-zero exit means at least one subject failed the style check AND no saved message was available — surface as a warning to the user but do not abort.

Step 8: Merge to Main

Move the cleaned-up commits to main:

git checkout main
git merge <branch> --ff-only

If --ff-only fails, use AskUserQuestion:

Fast-forward merge failed — main has diverged from <branch>.

○ Rebase — rebase <branch> onto main first (recommended)
○ Merge commit — create a merge commit instead
○ Abort — stop and let me handle it

If "Rebase": run git checkout <branch> && git rebase main, then retry the ff merge. If rebase has conflicts, report and stop.

If "Merge commit": run git merge <branch> (allow merge commit).

If "Abort": stop.

Step 9: Optional Tag

Only if --tag <version> was provided:

git tag <version>

Never prompt for a tag. Never auto-tag.

Step 10: Validate and Cleanup

Invoke the validate-commits skill to run all five checks.

After validation passes, ask before cleaning up using AskUserQuestion:

Branch <branch> has been merged to main. Delete it?
○ Yes — delete the branch
○ No — keep it

If "Yes":

git branch -d <branch>

If a worktree was used (check via git worktree list):

git worktree remove <path>

Soft-Reset Escape Hatch

If the user chose "Reset" in Step 5, instead of interactive rebase:

git reset --soft $base

This unstages all changes. Group files by top-level directory and concern, then present groupings via AskUserQuestion. For each group:

1. Stage the group's files: git add <files>
2. Invoke /commit

The hygiene findings from Step 4 carry forward — display them again as context for grouping decisions.

Main Flow (on main/master)

Step 1: Verify Tests

Same as Branch Flow Step 1.

Step 2: Identify Unpushed Commits

Try to find commits ahead of upstream:

git log --oneline @{u}..HEAD 2>/dev/null

If this succeeds, show the commits.

If it fails (no upstream), use AskUserQuestion:

No upstream tracking branch. Which commit is the base?
○ Use latest tag (<tag from: git describe --tags --abbrev=0>)
○ Enter a commit ref

If --base was provided, use that instead of detection.

Step 3: Review Options

Use AskUserQuestion:

What would you like to do?
○ Validate only — check for issues without changing commits
○ Rebase — run hygiene analysis and interactive rebase to clean up
○ Reword — amend commit messages using saved style

Validate only: Invoke the validate-commits skill. Done.

Rebase: Run the same analysis and rebase workflow as Branch Flow Steps 4-7:

1. Ensure Codebase Index (set $SEMANTIC_AVAILABLE)
2. Run Commit Hygiene Analysis against $base..HEAD
3. Plan and review (Steps 4-5 of Branch Flow)
4. Execute (Step 6 of Branch Flow)
5. Revalidate (Step 7 of Branch Flow)

All steps work identically to the Branch Flow — the only difference is that there is no merge step afterward (commits are already on main).

Reword: Walk through commits oldest-to-newest. Let the user select which commits to reword via AskUserQuestion. Build a plan where all selected commits have action: reword and all others have action: pick. Execute via the same Steps 4-7 of Branch Flow (the synthesizer authors the new messages, the executor replays them, the revalidator confirms style).

Step 4: Optional Tag

Same as Branch Flow Step 9.

Step 5: Validate

Invoke validate-commits (unless "Validate only" was already selected in Step 3, which already ran it).

Failure Handling

Failure	Response
A reader subagent fails on one commit	Synthesizer treats that commit as pick and adds a note. The user sees the note in plan review and can override the action.
codebase-memory-mcp unavailable	Skip Layer 3 entirely. Layers 1+2 still run. Set $SEMANTIC_AVAILABLE=false.
Synthesizer cannot author a message for an action	Surface the gap before plan review. The user chooses: accept the original message (downgrades the action to pick) or abort the run.
Rebase conflict during execute	Stop, list conflicting files, await git rebase --continue. Resume at Step 7 once the rebase completes.
Revalidator finds a drifted subject AND has a saved message	Auto-amend with the saved message. No user intervention.
Revalidator finds a drifted subject AND has NO saved message	Print a warning to stderr listing the drifted hash and subject. Do not abort.
Revalidator finds a planned action that did not materialize (missing hash, wrong split count)	Warn loudly to the user. Do not auto-fix; the user must decide whether to redo the run.

Integration

With /commit

This skill READS plugins/commit-tools/styles/<style>.md directly during the synthesis phase to author messages in the saved style. It does NOT invoke /commit at execute time — the executor is purely mechanical replay. /commit is a peer of this skill, not a caller.

With /codebase:index

This skill invokes /codebase:index to build or refresh the codebase-memory-mcp index when needed. The index enables Layer 3 semantic analysis for commit hygiene checks.

With Superpowers

• finishing-a-development-branch detects this skill when "Merge locally" is selected
• subagent-driven-development naturally leads here after tasks complete

Standalone

Run /review-commits anytime — before pushing, after a messy session, to clean up agent commits.

What This Skill Does NOT Do

• Generate commit messages at execute time — messages are pre-authored in the planning phase
• Open $EDITOR mid-rebase — GIT_EDITOR=true is always set
• Auto-tag — only with explicit --tag
• Add co-author — never
• Push — user pushes manually
• Resolve merge conflicts — reports and stops
• Force semantic analysis — if codebase-memory-mcp is not available, Layers 1-2 still provide value