SuperAPI AI Directory

review-branch

Coding & development

View repository

Updated 10-16 06:49

About this skill

review-branch — local convergence, no PR required

Drive a branch to a clean state by iterating review → apply → simplify → re-review, with every finding carried in a local file instead of a GitHub review thread.

This is review-loop with the GitHub round trip removed. The findings are the same findings — the branch-reviewer agent runs the same detection core pr-reviewer does, by reference rather than by copy — but nothing is posted, nothing is fetched, and the loop's state lives in .agent/{branch}/findings.jsonl.

When to use which:

review-branch review-loop
Needs a PR no yes
Bus local findings.jsonl GitHub review threads
GitHub calls zero many
Green gate the repo's own fast checks CI
Right when iterating on your own work, pre-PR, offline a PR exists and other people read the threads

The thread is not overhead once someone else is reading it — it is the record, and it is how a human reviewer sees what the agent already handled. Reach for review-loop the moment a PR exists.

Modes

Flag Effect
--base <ref> Base of the comparison. Default: upstream → origin/HEAD → main, and the loop announces which rung answered.
--head <ref> Head of the comparison. Default: the working tree, so uncommitted work is reviewed.
--cap N Iteration cap. Default 5.
--effort high Forces the deep tier and raises diversify-then-vote from 3 finders to 5.
--no-simplify Skip sub-step C (polish simplify).
--no-checks Skip sub-step D. Convergence then means findings-clean only, and the report says so.
--report Report-only. Forces CAP=1, skips B/C/D, applies nothing. The local counterpart of review-loop's --no-feedback.
--include-untracked Treat untracked, non-ignored files as added. Off by default.

Dispatch mechanics

branch-reviewer is an agent. Dispatch it with the harness's sub-agent dispatch tool; never Skill("branch-reviewer", …), which errors with Unknown skill.

The check is a capability, never a tool name. Task is the Claude Code CLI's spelling and Agent is the Claude Agent SDK's; a tool taking a subagent_type (or equivalent) parameter is the capability whatever it is called.

# WRONG — a name check. Skips the review on every harness that spells it `Agent`.
if "Task" not in available_tools: skip

# RIGHT — a capability check, name-agnostic.
if no available tool dispatches a sub-agent (Task, Agent, or another spelling): skip

Absent dispatch is a skip, not a fallback. Reviewing in this context would be a self-review wearing a reviewer's label — the same reasoning pr-review applies. Report it and stop:

- [TIMESTAMP] review-branch — skipped (sub-agent dispatch unavailable; branch-reviewer requires it)

One absent-dispatch return is conclusive; never retry. And this loop, like review-loop, must run at the top level — its first sub-step is a delegation, so dispatching the loop itself into a sub-agent spends the budget one rung too high and leaves it nothing to review with.

Procedure

Step 0 — resolve the range and preconditions

BRANCH=$(git rev-parse --abbrev-ref HEAD)
BASE=${BASE_FLAG:-$(git rev-parse --abbrev-ref --symbolic-full-name '@{upstream}' 2>/dev/null \
      || git symbolic-ref --quiet --short refs/remotes/origin/HEAD 2>/dev/null \
      || echo main)}
BUS=".agent/${BRANCH}/findings.jsonl"
mkdir -p "$(dirname "$BUS")"

Abort before any work if the range is empty — git diff --quiet "$BASE" — with review-branch: no changes against <base>. An empty range is never a pass; reporting one as a converged branch is the local spelling of green-washing.

Do not create the bus file yet. Its absence is how iteration 1 knows FINDING_OVERLAP is 0.

Step 1 — the loop

ITERATION = 0
APPLIED = DECLINED = FLAGGED = 0
CHECKS  = "unread"
STOP_REASON = "cap-reached"    # only correct if the WHILE CONDITION ends the loop;
                               # every break below overwrites it. Never re-derive a
                               # stop reason from the iteration count — a report-only
                               # run and a last-iteration convergence look identical.

while ITERATION < CAP:
    ITERATION += 1

    # A — review. Always first, so a pass validates the previous iteration's fixes
    #     before anything else touches the tree.
    <dispatch>(subagent_type="branch-reviewer",
               prompt="--base $BASE --out $BUS <flags>")
    # The FULL range every iteration, never narrowed to "since the last pass".
    # That is what lets this pass validate the previous one's fixes, and the bus —
    # keyed by fingerprint, not by line — is what keeps the re-review from
    # re-reporting what it already saw. A narrowed range would do the opposite:
    # hide a regression the last iteration's fix introduced outside the new delta.
    NEW = findings raised this pass that are not already in the bus

    if REPORT_ONLY:
        STOP_REASON = "report-only"; break

    if NEW == 0 and open_findings(BUS) == 0 and checks_are_green():
        STOP_REASON = "converged"; break

    open_before = open_findings(BUS)

    # B — apply. In-context, one commit per finding, highest severity first.
    #     Each finding leaves `open` through a fix OR a written rationale.
    apply_findings(BUS)          # → appends `applied` / `declined` / `flagged`

    # C — simplify
    if not NO_SIMPLIFY:
        Skill("polish", "simplify")

    # D — the local green gate
    if not NO_CHECKS:
        CHECKS = run_fast_checks()        # green | red | none
        if CHECKS == "red":
            fix the regression this loop just introduced, then re-run once
            if still red: STOP_REASON = "checks-red"; break

    if APPLIED_THIS_ITER == 0 and DECLINED_THIS_ITER == 0
       and open_findings(BUS) >= open_before:
        STOP_REASON = "no-progress"; break

Sub-step B — apply, in this context

Unlike review-loop, the apply runs here, not in a worker sub-agent. There is no bus to hand a worker and no thread for it to resolve, and the round trip buys nothing once the findings are already on disk.

That is safe in a way the review is not: applying a finding is not judging it. The judgment already happened in an isolated context, and its verdict, score, and severity are in the bus and reviewer-immutable.

Work highest severity first. Per finding:

  1. Read the finding's path:line and its evidence[] in the code, not from the record.
  2. Apply the smallest change that addresses the claim. Never widen it.
  3. Commit it alone, so a bad fix reverts without taking a good one with it.
  4. Append the new state — applied, declined with a note, or flagged — per findings-bus.md.

A finding you disagree with is declined with a rationale, never deleted. A finding that is real and out of scope is flagged, never declined. If you cannot write an honest rationale, that is the signal you should be flagging it.

Sub-step D — the local green gate

CI's local analogue. Run the repo's own fast checks — the ones a contributor runs before pushing — discovered in this order, first hit wins:

  1. A check / verify / precommit script in package.json, Makefile, justfile, or Taskfile.
  2. The lint + typecheck + test targets the repo documents in CLAUDE.md / AGENTS.md / README.md.
  3. Nothing found ⇒ CHECKS = "none", which counts as green for convergence and is reported as none rather than green — a loop that ran no checks must not claim a clean build.

Scope them to the touched packages where the repo supports it; run them unscoped at least once before the loop exits, because a scoped pass cannot see a consumer the edit broke elsewhere. That is the same lesson implement-suggestion's Phase 6 pre-push gate encodes.

Red checks are this loop's own mess to clean. There is no ci-auto-fix handoff here: nothing was pushed, so a red check is a regression this loop just wrote. Fix it, or revert the commit that caused it. Never weaken a check, skip a test, or push past it — every ci-auto-fix anti-pattern holds.

Step 2 — report

review-branch on <branch> (<base>..<head>)

Iterations: <N> of <CAP>
Stop reason: <converged | no-progress (flags remain) | cap-reached | checks-red
              | report-only (--report) | skipped (sub-agent dispatch unavailable)>
Review: branch-reviewer, tier <deep|standard|quick>, intent <checked vs plan.md | not run>

Per-iteration:
  1: <F found>, <A applied>, <D declined>, <S simplify recipes>, <O still open>
  2: ...

Findings: <A> applied · <D> declined · <G> flagged · <K> deferred
Flagged (needs you):
  - <path:line> — <title> — <why the loop could not resolve it>

Checks at exit: <green | red (<failing>) | none found | skipped (--no-checks)>
Bus: .agent/<branch>/findings.jsonl
Head: <sha>

Report the STOP_REASON the loop set. Never infer it from the iteration count — 1 of 1 is what report-only, a first-iteration convergence, and a --cap 1 run all look like from outside.

Surface every flagged finding prominently. They are the reason a human is still needed, and a run that buries them has converted the safety valve back into a green-wash.

Hard rules

  • Zero GitHub calls. Not gh, not mcp__github__*, in this skill or the agent it dispatches. That promise is the whole point; a run that quietly reaches for gh is a review-loop run with a misleading name.
  • The review is dispatched; the apply is not. Reviewing in this context is a self-review wearing a reviewer's label. Applying in this context is fine — the judgment already happened elsewhere.
  • The dispatch precondition tests a capability, never the literal name Task.
  • Absent dispatch is a skip, never an in-context review, and one such return is conclusive.
  • Never green-wash. A finding leaves open only via a fix that landed or a rationale that was written. Never by deletion, never at the cap, never to reach zero.
  • flagged is reported, always. Converging with flags is an honest outcome; hiding them is not.
  • An empty diff is not a pass. Report no changes and exit.
  • Never rewrite the bus. Append; the last record for an fp is its state (findings-bus.md).
  • Never edit a reviewer-written field. Claim, score, severity, and verdict are immutable to this loop — the same discipline checks.yaml applies to the executor.
  • Only Skill("polish", "simplify"). Any other polish mode dispatches a reviewer pass and creates a cycle — the same anti-circularity guarantee review-loop holds.
  • Cap is a hard limit. Surface what is open and stop; never extend it silently.
  • Hand off at the PR boundary. Once a PR exists, review-loop owns convergence. Do not re-run this loop against a branch whose PR is open and being read.

Relationship to other skills

Skill Relationship
branch-reviewer Sub-step A — the PR-less reviewer, dispatched per iteration. Composes the detection core by reference.
review-loop Sibling, never nested. Same loop shape over a different bus. This one runs pre-PR; that one runs once a PR exists.
polish Sub-step C (simplify only). polish's own review pass needs a PR; this loop is what makes a pre-PR review possible at all.
create-pr Caller, at its pre-push Step 5.5 — default-on in both modes (--no-pre-review opts out), and load-bearing under --split, where its post-draft review-loop cannot run at all and the slot otherwise gets polish simplify with no review. Converge here, then open the PR with less left for the post-draft loop to find.
pr-review The one-shot read-only counterpart, for a PR. --report is this skill's equivalent for a branch.
findings-bus.md Owns the record, the lifecycle, and the convergence predicate. Read it; never restate it.