# Review Loop

> Bounded review-apply-resolve convergence loop for a GitHub PR (draft PRs are fine). Runs up to N=5 iterations of pr-reviewer → implement-suggestion (--resolve-all) → polish simplify, converging until every review thread is resolved — through a fix OR a reply (answered question, recorded rationale) — so the PR is left with zero open threads (only genuine human-judgment flags stay open). On convergence it also refreshes the PR description to match the shipped diff and, best-effort, notes the linked Linear ticket. Use after opening a draft PR to converge the branch to a clean, review-ready state before undrafting. Also converges CI: after each iteration's push it reads the check state and delegates a red mechanical failure to ci-auto-fix, so convergence means zero open threads AND CI not red (--no-ci opts out; create-pr and autonomous-workflow pass it because they own their own CI phase). On a UI PR it also runs the committed preview-spec block against the live preview deployment once at exit (report-only, never

- Skill: `mthines/review-loop` (Agent Skill)
- Install (CLI): `npx skillmds@latest add mthines/review-loop`
- Raw SKILL.md: https://api.skillmd.com/api/skills/mthines/review-loop/raw
- Safety review: pending
- Works with: Claude Code, Claude.ai, OpenAI Codex
- Category: Docs & Writing
- License: MIT
- Author: mthines (https://skillmd.com/u/mthines)
- Updated: 2026-09-17
- Page: https://skillmd.com/skills/mthines/review-loop

---


# review-loop — Bounded Review-Apply-Resolve Convergence

Drive a PR from its initial draft state to a clean, review-ready state by
iterating `pr-reviewer` → `implement-suggestion --resolve-all` → `polish simplify`
until **every review thread is resolved** or the cap is reached, then refresh the
PR description to match the shipped diff.

A thread is resolved when it is **either fixed** (a code change landed) **or
answered** (a reply — the answer to a question, the agent's take on a discussion,
or a rationale for a declined suggestion). The only threads left open at
convergence are genuine **human-judgment flags**: a real potential issue the
agent will neither auto-apply nor honestly decline. That safety valve means the
loop can never green-wash a PR by resolving a live finding — it surfaces it
instead.

This skill is an **orchestrator**.
It contains no quality rules of its own.
It sequences existing pieces, each owning its own domain:

1. `pr-reviewer` — finds issues (read-only; posts one `COMMENT` review; on a re-review resolves its own addressed threads).
2. `implement-suggestion --resolve-all` — applies actionable findings **and** replies-to-and-resolves the non-fix threads it can honestly close (single-shot, no `--watch`).
3. `Skill("polish", "simplify")` — applies Class M mechanical refactors behind a confidence gate.
4. `ci-auto-fix` — diagnoses and fixes a red check after the iteration's push (skipped under `--no-ci`).
5. On convergence — refreshes the PR description (via the shared description-contract) and, best-effort, notes the linked Linear ticket.

Under `--external-review`, step 1 is replaced by a **wait**: the reviewer is
another process (a review bot, a CI-triggered agent, a teammate), and the loop
polls for its output instead of producing its own. Steps 2–5 are unchanged —
they consume threads from GitHub and do not care who wrote them.

### Dispatch mechanics — read before invoking

`pr-reviewer` is an **agent**, not a skill. Dispatch it with the harness's
sub-agent dispatch tool (`Task(subagent_type="pr-reviewer", prompt="<PR-URL> [--critical]")`).
**Do not** call `Skill("pr-reviewer", …)` — there is no skill by that name and it
errors with `Unknown skill: pr-reviewer`.

#### The dispatch tool is a capability, not a fixed name

Harnesses spell that tool differently. `Task` is the Claude Code CLI's name for
it; the Claude Agent SDK harness behind Claude Code on the web and in cloud
sessions names it `Agent`; other hosts may add further spellings. **Every
capability check in this file therefore asks whether *any* sub-agent dispatch
tool is present, never whether one specific name is.** The names above are
examples of the capability, not its definition.

```text
# WRONG — a name check. In a harness that spells the tool `Agent` this concludes
# "no dispatch available" and skips the review on a PR that was reviewable.
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): degrade
```

This is not cosmetic: the whole degradation ladder below hangs off this one
check, so a false negative costs the PR its review.

#### Caller contract — run this loop at the top level, never inside a sub-agent

This skill is an orchestrator whose **first sub-step is itself a delegation**. It
must therefore be invoked from a context that still holds a dispatch tool. Most
harnesses give a dispatched sub-agent no dispatch tool under any name (Dash0
Agent0 sub-agents cannot delegate further, by platform design; a Claude Agent
SDK sub-agent has neither `Task` nor `Agent`), so a caller that
dispatches this loop into a sub-agent spends the run's delegation budget one level
too high and leaves the loop with nothing to dispatch `pr-reviewer` with. The loop
then has exactly one honest outcome: a skip at iteration 0, with the PR unreviewed.

```text
# WRONG — the loop arrives with no dispatch tool and can only skip at iteration 0
Task(subagent_type="general", prompt="Run /review-loop <PR-URL>")

# RIGHT — the caller runs the loop itself and spends its dispatch budget on the
# agents the loop actually needs
Skill("review-loop", "<PR-URL>")        # → the loop dispatches pr-reviewer itself
```

A caller that can make **only one** dispatch has two supported shapes, in
preference order:

| Shape | What the caller does | Consequence |
| --- | --- | --- |
| **Own the loop** (preferred) | Run this procedure at the top level and spend the delegation budget on `pr-reviewer` / `implement-suggestion` | The only shape in which the loop can converge a PR |
| **Delegate with `--external-review`** | Dispatch the loop *with `--external-review` passed deliberately by the caller*, never invented by the callee | No `pr-reviewer` pass happens: a fix-and-polish loop over someone else's review |

**One skip is conclusive — never retry the dispatch.** An absent dispatch tool is
a property of the dispatch topology, decided before any code is read; a second
attempt re-derives a platform fact at the cost of a full round trip and cannot
change the outcome.

**When sub-agent dispatch is unavailable.** Some harnesses expose no dispatch
tool at all, so that dispatch fails outright (`Failed to run agent`). `pr-reviewer` has
**no `Skill()` form and no in-context substitute** — its review independence comes
from running in a fresh, isolated context, so "play the role yourself" would
produce a self-review wearing a reviewer's label, which is worse than no review.

Check for it in [Step 0](#step-0-resolve-the-pr-and-preconditions) and **self-report
a clean skip** rather than letting the caller discover it as a mid-loop tool error:

Two causes produce the same absent capability, and they get **different skip
lines** because they have different fixes. Report the one you can evidence; when
you cannot tell them apart, report the harness line:

| Cause | How you know | Skip line |
| --- | --- | --- |
| **Nested dispatch** (caller error, fixable today) | You are running as a dispatched sub-agent — the caller's prompt dispatched this loop rather than running it | `skipped (nested dispatch — review-loop must run at the top level; the caller consumed the delegation budget)` |
| **Harness exposes no dispatch tool** (environment) | This is the top-level session and no tool that dispatches a sub-agent is present under any name — `Task`, `Agent`, or another spelling | `skipped (sub-agent dispatch unavailable; pr-reviewer requires it)` |

```markdown
- [TIMESTAMP] review-loop — skipped (nested dispatch — review-loop must run at the top level; the caller consumed the delegation budget). Have the caller run the loop itself, or dispatch it with --external-review.
- [TIMESTAMP] review-loop — skipped (sub-agent dispatch unavailable; pr-reviewer requires it)
```

Return that skip as the loop's terminal result. Do **not** retry the dispatch and
do **not** silently continue to sub-steps B and C — without a review pass there are
no findings to apply, and running `polish simplify` alone would misreport an
unreviewed PR as converged.

**`--external-review` is the exception, and the graceful-degradation path.** In
that mode the loop never dispatches `pr-reviewer`, so this precondition does not
apply and **must not** fire: the review comes from another process that has
already written to GitHub. A harness with no dispatch tool can therefore still
run the loop — suggest `--external-review` in the skip line rather than presenting
the skip as the only outcome:

```markdown
- [TIMESTAMP] review-loop — skipped (sub-agent dispatch unavailable; pr-reviewer requires it). Re-run with --external-review if another agent reviews this PR.
```

One caveat to state plainly: sub-step B (`implement-suggestion`) dispatches a
**worker** subagent of its own, which also wants a dispatch tool. Its documented inline
fallback (apply commit-per-comment, push, reply-and-resolve yourself) covers that
case — see the paragraph below. `--external-review` removes the `pr-reviewer`
dependency, not every sub-agent dependency.

The check is best-effort, not certain: there is no capability-introspection API, and
a refused dispatch may surface as an uncatchable harness error. Its value is
**placement** — one clean logged deviation at Step 0 instead of a mid-Phase-6 error
the caller has to interpret.

`implement-suggestion` and `polish` **are** skills — invoke them with `Skill(...)`.
If a given install has `implement-suggestion` set `disable-model-invocation: true`
(so `Skill("implement-suggestion")` is refused), fall back to applying its
contract inline: resolve a worktree at the PR head, apply the findings as
commit-per-comment, push, and reply-to-and-resolve the threads yourself (the
same work the skill's worker does) — never skip sub-step B silently.

## Modes

Parse the **first positional argument** as the PR reference.
Everything else is a flag.

| Flag | Effect |
| --- | --- |
| `--cap N` | Override the default iteration cap of 5. |
| `--critical` | Pass `--critical` to each `pr-reviewer` call (adversarial pre-mortem). |
| `--no-feedback` | Report-only. Forces `CAP=1` and skips sub-steps B, C, and the final refresh, so `pr-reviewer` runs once and its findings are reported without being applied, resolved, or pushed. |
| `--no-refresh` | Run the convergence loop as normal but skip the final PR-description refresh and Linear note. |
| `--external-review` | Replace sub-step A: wait for an **out-of-process** reviewer instead of dispatching `pr-reviewer`. See [Sub-step A — external-review mode](#sub-step-a--external-review-mode). |
| `--interval S` | Poll interval in seconds for `--external-review`, default `300`, **clamped to `540`**. Ignored without `--external-review`. |
| `--no-ci` | Skip sub-step D (the CI pass). Callers that own their own CI phase pass this — `create-pr` (Steps 7–9) and `autonomous-workflow` (Phase 7) both do. |
| `--no-preview-run` | Skip [Step 1.6](#step-16-preview-spec-run-report-only-once-on-exit), the report-only preview-spec run at exit. `autonomous-workflow` passes this because its Phase 7 spec rehearsal already runs the same specs against the preview; `create-pr` does **not**, so a hand-driven UI PR gets its authored spec verified here. |

**Incompatible combinations**, refused or downgraded at Step 0:

| Combination | Behaviour |
| --- | --- |
| `--external-review` + `--no-feedback` | **Refuse.** `--no-feedback` means "run `pr-reviewer` once and report"; with no `pr-reviewer` there is nothing to report. Print `--no-feedback needs pr-reviewer; drop --external-review or drop --no-feedback.` and exit. |
| `--external-review` + `--critical` | **Warn and ignore.** `--critical` only ever fed `pr-reviewer`. Print one line noting it was ignored, then continue — callers pass it by habit and it must not abort the run. |

## Procedure

### Step 0: Resolve the PR and preconditions

```bash
# Resolve PR number and repo from the argument
# (mirrors the parsing logic in pr-reviewer Step 0)
if [[ "$ARG" =~ ^https://github\.com/([^/]+/[^/]+)/pull/([0-9]+) ]]; then
  PR_REPO="${BASH_REMATCH[1]}"
  PR_NUMBER="${BASH_REMATCH[2]}"
elif [[ "$ARG" =~ ^#?([0-9]+)$ ]]; then
  PR_REPO=""
  PR_NUMBER="${BASH_REMATCH[1]}"
fi

RESOLVED_REPO=${PR_REPO:-$(gh repo view --json nameWithOwner -q .nameWithOwner)}
OWNER="${RESOLVED_REPO%/*}"
REPO="${RESOLVED_REPO#*/}"
```

If no PR reference is found, abort: `review-loop requires a PR URL or #<n>.`

**Precondition — sub-agent dispatch (best-effort).** The loop's first sub-step
dispatches the `pr-reviewer` agent, which has no in-context substitute (see
[Dispatch mechanics](#dispatch-mechanics--read-before-invoking)). Before entering
the loop, check for the **capability**, not a name: does any available tool
dispatch a sub-agent?

1. Scan your available tools for one whose job is dispatching a sub-agent —
   `Task` and `Agent` are the two spellings in circulation, and a tool that takes
   a `subagent_type` (or equivalent agent-name) parameter is one whatever it is
   called.
2. Found one → **proceed**, and dispatch `pr-reviewer` through it. Substitute its
   name wherever this file writes `Task(...)`; the call shape is otherwise
   identical.
3. Found none → emit the skip line from that section and return, without running
   sub-steps B or C. Pick the line by cause — **nested dispatch** when you are
   running as a dispatched sub-agent, the harness line otherwise — and do **not**
   retry: one absent-capability return is conclusive.

**Never conclude "no dispatch" from the absence of the single name `Task`.** That
misread is what this step exists to prevent: the harness behind Claude Code on
the web names the tool `Agent`, so a `Task`-only check skips the review on every
cloud session, reports the PR as unreviewable, and the failure is invisible
because a skip is a legitimate outcome.

**Skip this precondition entirely when `--external-review` is set** — that mode
dispatches no `pr-reviewer`, so an absent dispatch tool is not disqualifying.

**This check cannot be made certain**, and the contract does not pretend otherwise:
there is no capability-introspection API, and on some harnesses a refused dispatch
surfaces as an uncatchable error rather than a return value. When the check is
inconclusive, attempt the dispatch — and if it fails, emit the same skip line rather
than retrying or working around it. The value is **placement**: one clean logged
deviation instead of a mid-Phase-6 error the caller must interpret.

Parse the flags and set the iteration cap:

```bash
# Parse flags out of the argument string.
cap_flag=""
CRITICAL=0
NO_FEEDBACK=0
NO_REFRESH=0

# --cap N: override the default iteration cap (accepts "--cap 5" or "--cap=5").
if [[ " $ARGUMENTS " =~ [[:space:]]--cap[[:space:]=]+([0-9]+) ]]; then
  cap_flag="${BASH_REMATCH[1]}"
fi

# --critical: pass the adversarial pre-mortem through to each pr-reviewer call.
if [[ " $ARGUMENTS " == *" --critical "* ]]; then
  CRITICAL=1
fi

# --no-refresh: skip the final PR-description refresh + Linear note.
if [[ " $ARGUMENTS " == *" --no-refresh "* ]]; then
  NO_REFRESH=1
fi

# --external-review: sub-step A waits for an out-of-process reviewer.
EXTERNAL_REVIEW=0
if [[ " $ARGUMENTS " == *" --external-review "* ]]; then
  EXTERNAL_REVIEW=1
fi

# --interval S: poll interval for --external-review. Clamp to 540 (below the
# 600 s Bash tool cap) exactly as watch-mode does; values above are clamped
# silently.
INTERVAL=300
if [[ " $ARGUMENTS " =~ [[:space:]]--interval[[:space:]=]+([0-9]+) ]]; then
  INTERVAL="${BASH_REMATCH[1]}"
fi
[ "$INTERVAL" -gt 540 ] && INTERVAL=540

# --no-ci: skip sub-step D. Callers owning their own CI phase pass this.
NO_CI=0
if [[ " $ARGUMENTS " == *" --no-ci "* ]]; then
  NO_CI=1
fi

# --no-preview-run: skip Step 1.6, the report-only preview-spec run at exit.
# autonomous-workflow passes this (its Phase 7 rehearses the same specs).
NO_PREVIEW_RUN=0
if [[ " $ARGUMENTS " == *" --no-preview-run "* ]]; then
  NO_PREVIEW_RUN=1
fi

CAP=${cap_flag:-5}
ITERATION=0

# --no-feedback degrades the loop to a single read-only review pass.
if [[ " $ARGUMENTS " == *" --no-feedback "* ]]; then
  NO_FEEDBACK=1
  CAP=1
  NO_REFRESH=1
fi

# Refuse the one combination that cannot mean anything: report-only needs a
# reviewer to report, and --external-review removes the only one this loop owns.
if [ "$EXTERNAL_REVIEW" -eq 1 ] && [ "$NO_FEEDBACK" -eq 1 ]; then
  echo "--no-feedback needs pr-reviewer; drop --external-review or drop --no-feedback."
  exit 1
fi

# --critical only ever fed pr-reviewer. Warn, do not abort — callers pass it by habit.
if [ "$EXTERNAL_REVIEW" -eq 1 ] && [ "$CRITICAL" -eq 1 ]; then
  echo "note: --critical ignored under --external-review (it only configures pr-reviewer)."
  CRITICAL=0
fi
```

> **Naming.** `NO_FEEDBACK` here is the **report-only mode flag** (`--no-feedback`).
> The shared review-activity poll emits an outcome string also spelled
> `NO_FEEDBACK`, meaning "no new review activity this interval". They are
> unrelated. Sub-step A below reads the poll's result into `POLL_RESULT`
> (`new` / `quiet` / `error`) and never into this variable.

A helper for the exit check — the count of **unresolved** review threads:

```bash
unresolved_thread_count() {
  gh api graphql -f query='
    query($owner:String!,$repo:String!,$pr:Int!){
      repository(owner:$owner,name:$repo){
        pullRequest(number:$pr){
          reviewThreads(first:100){ nodes{ isResolved } }
        }
      }
    }' -F owner="$OWNER" -F repo="$REPO" -F pr="$PR_NUMBER" \
    --jq '[.data.repository.pullRequest.reviewThreads.nodes[] | select(.isResolved==false)] | length'
}
```

### Step 1: Loop — review → apply+resolve → simplify

Each iteration runs up to four sub-steps.
The loop exits when **every review thread is resolved** (`unresolved_thread_count == 0`)
**and CI is settled** (green, pending, or absent — never red), when an iteration makes
**no progress** (the only threads left are ones nothing can resolve — human-judgment
flags), or at the cap.

When `NO_FEEDBACK == 1`, only sub-step A runs: sub-steps B and C are skipped, the
push and the refresh are skipped, and the run reports the findings without applying
anything.

```text
APPLIED_TOTAL = 0
CI_HANDOFFS   = 0
CI_STATE      = "unread"     # no check state observed yet this run
STOP_REASON   = "cap-reached"  # the default is only correct if the WHILE CONDITION
                               # ends the loop; every break below overwrites it.
                               # ITERATION == CAP is NOT the cap test — a run that
                               # converges (or is report-only with CAP forced to 1)
                               # exits on its last allowed iteration too.
while ITERATION < CAP:
    ITERATION += 1

    # Sub-step A: review — always the FIRST thing each iteration runs, so a
    # review pass validates the previous iteration's fixes and resolves this
    # agent's now-addressed threads before anything else touches the PR.
    #
    # Two of the four exits below therefore land on a review pass — the
    # report-only break and the clean-convergence exit, both immediately after
    # this sub-step. The other two do NOT: the no-progress guard fires at the
    # bottom of the body (after B/C/D) and the cap fires at the loop condition,
    # so in both the last thing that ran was a push, not a review. Report those
    # exits as what they are; never describe them as validated by a final review.
    if EXTERNAL_REVIEW == 0:
        review = <dispatch>(subagent_type="pr-reviewer",
                      prompt="<PR-URL>" + (" --critical" if CRITICAL == 1 else ""))
        # <dispatch> is the harness's sub-agent dispatch tool — Task, Agent, or
        # another spelling; Step 0 resolved which one. pr-reviewer is an AGENT,
        # so never Skill("pr-reviewer").
        # On a re-review it resolves its own addressed threads (thread-resolution.md).
        NEW_FINDINGS = (pr-reviewer reported new actionable findings)
    else:
        POLL_RESULT = shared review-activity poll, bounded by INTERVAL   # new | quiet | error
        if POLL_RESULT == "error":
            abort → stop reason "poll error"      # a broken probe is NEVER "quiet"
        if POLL_RESULT == "quiet" and ITERATION > 1:
            NEW_FINDINGS = false                  # reviewer silent → fall to the exit below
        else:
            NEW_FINDINGS = true                   # iter 1 always runs a pass

    if NO_FEEDBACK == 1:
        STOP_REASON = "report-only"
        break   # report-only: never apply, never resolve, never simplify, never push

    # CLEAN CONVERGENCE EXIT — the only exit that means "done":
    # ci_is_settled() reads check state on demand when CI_STATE is still "unread"
    # (iteration 1 can reach this exit before sub-step D has ever run), so the
    # loop can never converge on a build it has not looked at.
    if NEW_FINDINGS == false AND unresolved_thread_count() == 0 AND ci_is_settled():
        STOP_REASON = "all-threads-resolved"
        break   # every thread resolved (fix or reply), nothing new to fix, CI not red

    unresolved_before = unresolved_thread_count()

    # Sub-step B: apply findings AND resolve non-fix threads
    Skill("implement-suggestion", "<PR-URL> --resolve-all")
    # If this install has implement-suggestion set disable-model-invocation:true,
    # Skill() is refused — use the inline fallback from "Dispatch mechanics" above
    # (apply commit-per-comment, push, reply-and-resolve yourself). Never skip B.
    # Single-shot apply — no --watch; the loop drives re-review itself.
    # --resolve-all: fixes what it can, and replies-to-and-resolves questions /
    # discussions / declined suggestions; leaves only human-judgment flags open.
    APPLIED_TOTAL += (applies + answers this iteration, from its report)

    # Sub-step C: simplify
    Skill("polish", "simplify")
    # Applies Class M mechanical refactors; never runs the reviewer pass.

    push any local changes:
    git push

    # Sub-step D: CI. Read check state at the CURRENT REMOTE HEAD, then delegate
    # a red mechanical failure to ci-auto-fix. Skipped under --no-ci.
    if NO_CI == 0:
        CI_STATE = read check state (stateless query, no watch)   # green|pending|red|error
        if CI_STATE == "error":
            # Tooling failure, not "no CI" and not a red build. Same verdict as
            # ci_is_settled()'s error arm: never route to ci-auto-fix, never converge.
            STOP_REASON = "ci-error"
            break   # report the query failure verbatim and escalate
        if CI_STATE == "red" and CI_HANDOFFS < 2:
            dispatch ci-auto-fix as a subagent; CI_HANDOFFS += 1
            CI_STATE = "unread"   # the handoff pushed a fix, so the recorded red
                                  # describes a commit that is no longer head.
                                  # ci_is_settled()'s unread arm re-reads it.
        elif CI_STATE == "red":
            # Red with the handoff budget spent. Stop rather than spinning to the
            # cap: another review pass cannot fix a build ci-auto-fix already
            # failed twice on.
            STOP_REASON = "ci-red"
            break

    # No-progress guard: nothing was applied or answered AND the open-thread
    # count did not drop → the remaining threads are human-judgment flags the
    # loop cannot resolve. Stop early rather than spinning to the cap. (The clean
    # convergence exit above stays the normal path — it runs one more review pass
    # to validate before declaring done.)
    # CI is deliberately part of "progress": a red-CI iteration that fixed nothing
    # else still made progress if ci-auto-fix pushed, so the loop gets to re-review.
    if this iteration applied 0, answered 0, dispatched no ci-auto-fix,
       and unresolved_thread_count() >= unresolved_before:
        STOP_REASON = "no-progress"
        break

# Post-loop. Gate on STOP_REASON, never on ITERATION == CAP: report-only forces
# CAP=1 (so its break lands with ITERATION == CAP having pushed nothing), and a
# clean convergence on the last allowed iteration lands there too. Both were
# reported as "cap reached" by the old ITERATION == CAP test.
if STOP_REASON in ("cap-reached", "no-progress"):
    # Neither of these two exits ended on a review pass (see sub-step A), so the
    # state below is the state after the last PUSH — read it, do not assume it.
    if CI_STATE == "unread" and NO_CI == 0:
        CI_STATE = read check state   # never report a state you have not read at head
    if unresolved_thread_count() > 0 or CI_STATE == "red":   # CI_STATE stays "unread" under --no-ci
        report: <STOP_REASON>; surface remaining blockers/flags AND any red check
```

### Sub-step A — external-review mode

Under `--external-review` the loop produces no review of its own. It waits for one.

Run the shared [review-activity poll](../../../agents/shared/rules/review-activity-poll.md#the-poll)
with `SINCE` = the current baseline and `INTERVAL` as parsed at Step 0. That file
owns the procedure — call it, never restate it. Issue its Bash call with the tool
parameter `timeout: 600000`; the `--interval` clamp to 540 at Step 0 is what keeps
the loop's own bound reachable underneath it.

Map its [caller-neutral outcomes](../../../agents/shared/rules/review-activity-poll.md#outcomes-caller-neutral)
into `POLL_RESULT`:

| Poll outcome | `POLL_RESULT` | This loop does |
| --- | --- | --- |
| `NEW_FEEDBACK` | `new` | Run the iteration (sub-steps B, C, D) |
| `NO_FEEDBACK` | `quiet` | **Iteration 1:** run a pass anyway — the external reviewer may have reviewed before the loop started, and exiting here would converge a PR having done nothing. **Later iterations:** the reviewer is quiet; fall through to the convergence exit |
| `POLL_ERROR` | `error` | **Abort** with stop reason `poll error`. Report the stderr. A broken probe is never "the reviewer had nothing to say" — treating it as quiet would report a never-reviewed PR as converged |

**Advance the baseline after every pass**, exactly as the shared rule requires: set
`SINCE` to "now" once sub-steps B–D complete, so the next wait sees only what the
reviewer posted in response to the latest push. Leaving `SINCE` at its original
value re-reports the same review forever and the loop never reaches `quiet`.

What this mode does **not** change: sub-steps B, C, and D are byte-identical. They
read threads from GitHub and neither know nor care which process authored them.
`unresolved_thread_count()` is the same query, and the no-green-wash safety valve
is untouched — a live finding the agent cannot fix or honestly decline still stays
open, whoever raised it.

There is **no verdict** in this mode. `pr-reviewer`'s `PASS`/`FAIL` has no source
here, so the report prints `n/a (external review)` rather than inventing one.

### Sub-step D — CI

Skipped entirely when `--no-ci` is set.

After the iteration's push, read the check state **once** — stateless, at the
current remote head, no watch:

```bash
gh pr checks "$PR_NUMBER" --repo "$RESOLVED_REPO"
```

This is a **query, not a watch**: it adds no `gh … --watch` site and spends nothing
from the watch budgets that
[`create-pr` Step 9](../../delivery/create-pr/SKILL.md) and
[`phase-7-ci-gate.md`](../../workflow/autonomous-workflow/rules/phase-7-ci-gate.md)
each count inside their own invocation. `ci-auto-fix` likewise keeps its own local
counter, so delegating to it stays inside the existing contract — no budget is
shared, and none is carried across contexts.

Classify with the same three-way rule as `phase-7-ci-gate.md` Step 1 — **"no checks
reported" is three different states**, and a bare `gh pr checks` **exits non-zero
while merely pending**, printing to stdout, so non-zero with empty stderr means
"registered and running", not an error:

| Check state | `CI_STATE` | This loop does |
| --- | --- | --- |
| All terminal and passing | `green` | Nothing. Convergence may proceed |
| Any still pending | `pending` | Nothing this iteration — do **not** wait. A *continuing* loop re-reads it next iteration; a loop that exits here does not, so `pending` can be the state it converges on |
| Any check failing | `red` | Dispatch `ci-auto-fix` as a subagent (its output is loud and belongs out of this context), unless `CI_HANDOFFS` is already 2 |
| Query errored (exit 127, or stderr naming auth / network / rate limit / not-logged-in) | `error` | **Tooling failure, not "no CI".** Report and escalate. Never route to `ci-auto-fix` |
| Nothing reported, query succeeded, and this iteration just pushed | — | Not registered yet. Run the shared [registration poll](../../delivery/create-pr/rules/registration-poll.md#the-poll) and re-classify from its outcome; `no-ci` means this repo genuinely has no CI, and counts as `green` for convergence |

```text
ci_is_settled():   # the convergence predicate
    NO_CI == 1                      → true    # caller owns CI; not this loop's call
    CI_STATE == "unread"            → read check state now, then re-evaluate
    CI_STATE == "green"             → true
    CI_STATE == "pending", 1st time → re-read at head once, then re-evaluate
    CI_STATE == "pending", re-read  → true    # not red, and this loop never waits for CI
    CI_STATE == red                 → false
    CI_STATE == error               → abort, do not converge
```

The `"pending"` re-read is the same rule
[`watch-mode.md`](../../workflow/implement-suggestion/rules/watch-mode.md#ci-state-is-a-stop-reason-not-a-fix)
applies before its own stop, and it exists for the same reason: sub-step D reads seconds
after its own push, so `pending` is its usual answer, and converging on the first one means
converging on a build no check has finished. Exactly **one** re-read, and only at this
predicate — re-reading until a check is terminal would turn a loop that must never wait for
CI into a busy-wait on it.

The `"unread"` arm matters: iteration 1 can reach the convergence exit before
sub-step D has run even once (a PR that arrives already reviewed and thread-clean).
Without that arm the loop would report convergence having never looked at CI —
the precise failure this sub-step exists to prevent.

It is also the arm that keeps a `red` from going stale. A `ci-auto-fix` handoff
pushes a fix, so the `red` sub-step D just recorded describes a commit that is no
longer head; the handoff therefore resets `CI_STATE` to `"unread"`, and the next
`ci_is_settled()` re-reads instead of blocking convergence on a build that is
already fixed. The cap check does the same read for the same reason — the loop
never reports a CI state it has not read at the current head.

**Cap: 2 `ci-auto-fix` handoffs per `review-loop` run** (`CI_HANDOFFS`), matching the
per-PR cap the other two orchestrators use. Each handoff already burns a full internal
retry budget; do not wrap it in another loop. At the cap with CI still red, stop and
surface the failing checks — never extend it, and never converge a red PR silently.

**This loop never fixes CI itself.** It classifies and delegates. Every refusal in
[`ci-auto-fix`'s anti-patterns](../../delivery/ci-auto-fix/rules/anti-patterns.md)
holds transitively: no `--no-verify`, no `continue-on-error`, no skipped suites, no
weakened assertions to reach green.

**Hard rule: the only permitted `polish` invocation is `Skill("polish", "simplify")`.**
The `simplify` mode applies Class M mechanical refactors and dispatches no pr-reviewer.
All other `polish` modes trigger an internal agent pass, which would create a dispatch cycle.
This is the anti-circularity guarantee.

### Step 1.6: Preview-spec run (report-only, once, on exit)

After the loop exits — however it exited (converged, no-progress, or cap) — run the
PR's embedded preview-spec **once** against the live preview deployment. This is the
run half of the `author` step `create-pr` performs at its Step 6.4: the spec was
written into the PR body; here it is executed.

Skip this step entirely when **any** of:

- `NO_PREVIEW_RUN == 1` — the caller owns preview verification (`autonomous-workflow`
  passes this; its Phase 7 rehearses the same specs).
- `NO_FEEDBACK == 1` — report-only mode applied nothing, so there is nothing new to verify.
- the loop returned a dispatch skip (no dispatch tool, nested dispatch) — no run happened.

Otherwise dispatch it **once**, regardless of iteration count:

```text
Skill("preview-spec", "run <PR-URL>")
```

`preview-spec run` owns the whole procedure: it reads the committed
`<!-- preview-spec:v1 -->` block (the **only** source — never the gitignored
`.agent/{branch}/specs.md`, so it works on this or any checkout), resolves the
preview URL via the GitHub deployments API, dispatches `aw-tester --all`, and
returns a verdict. This loop only records the outcome.

**Deliberately invoked with no `--url`.** The deployments API has no
`mcp__github__*` equivalent, so on the `mcp` access path `preview-spec run`
cannot resolve a URL and returns `inconclusive: no access path for deployment
lookup (pass --url)` — the row below records it with an actionable note. This
loop does **not** resolve the URL itself: that is `preview-spec`'s own concern
([`preview-url-resolution.md`](../../testing/preview-spec/rules/preview-url-resolution.md)),
and a second implementation here would be the drift surface this repo argues
against. A caller that already holds a preview URL should run
`/preview-spec run <PR-URL> --url <preview-url>` directly instead.

**Report-only — this step never gates.** The verdict does **not** block convergence,
does **not** reopen the loop, and does **not** undraft the PR — matching
`autonomous-workflow`'s Phase 7 rehearsal, which also never auto-undrafts on the
spec verdict. Convergence is already decided by threads-resolved + CI-settled before
this step runs; the preview verdict is surfaced for the human undrafting the PR.

Map its outcome into the report:

| `preview-spec run` outcome | This loop records |
| --- | --- |
| `no spec` (no block — not a UI PR, or `author` never ran) | `not run (no preview-spec block)` — log and continue |
| `inconclusive: preview not deployed` | `inconclusive (preview not deployed at exit)` — note `re-run /preview-spec run <PR-URL> once the preview is up`. Never a red |
| `inconclusive: no access path for deployment lookup (pass --url)` | `inconclusive (no deployment lookup on this access path)` — note `re-run /preview-spec run <PR-URL> --url <preview-url>`. Never a red, and never recorded as `preview not deployed`: no lookup ran, so waiting for the build fixes nothing and only an explicit URL changes the outcome |
| any other `inconclusive: <reason>` (`preview building`, `no preview environment`, `preview deploy failed`, `preview URL not published`) | `inconclusive (<reason> at exit)` — log the reason verbatim and continue. Never a red |
| `empty spec` (markers present, body empty) | `not run (empty preview-spec block)` — log and continue. Distinct from `no spec` on purpose: `author` **did** run and embedded nothing, which is a spec-authoring bug worth naming, not a PR that needed no spec |
| `NOT RUN (<reason>)` (`chrome unavailable, user declined Playwright`, `sub-agent dispatch unavailable`, `no Chrome extension and no sub-agent dispatch available`) | `not run (<reason>)` — log the reason verbatim and continue. Never a red: no driver executed, so there is no verdict to be red about |
| `green` | `green (<N> specs on <preview-url>)` |
| `red` | `red (<N> failing on <preview-url>) — review before undrafting`. Report-only; does not reopen the loop |
| `preview-spec` not installed / `Skill()` refused | `skipped (preview-spec not available)` — log one line and continue; it is a non-load-bearing companion |
| anything else | `not run (unrecognised outcome: <verbatim>)` — quote what it returned and continue. An unmapped return is never recorded as `green` and never as a skip; the delegate gaining an outcome this table has no row for is exactly how a permanently-false note reached a report once already |

Run it **at most once** per `review-loop` invocation — it is an exit signal, not a
per-iteration check, and each run spends a full `aw-tester` Playwright dispatch.

### Step 2: Refresh the PR description and Linear note (on convergence)

Skip this step entirely when `NO_REFRESH == 1`, when `NO_FEEDBACK == 1`, or when
`APPLIED_TOTAL == 0` (the loop changed no code, so the description cannot have drifted).

Otherwise, refresh the PR body so it matches the diff that actually shipped after
the loop's fixes:

1. Regenerate the title and body following the shared
   [`description-contract.md`](../../delivery/create-pr/rules/description-contract.md)
   — the same contract `create-pr` uses, so the refresh keeps identical quality and
   length rules. Diff against the PR base and read the current body first; make it a
   minimal edit, not a rewrite.
2. Apply it:

   ```bash
   gh pr edit "$PR_NUMBER" --repo "$RESOLVED_REPO" --body "$(cat <<'EOF'
   <refreshed narrative body>
   EOF
   )"
   ```

Then, **best-effort**, note the linked Linear ticket (skip silently if any part is absent):

- Detect a ticket from the branch name (`.../ABC-123-...`), the PR title/body, or `gh pr view`.
- If a ticket id is found **and** the Linear MCP tools are connected, post a short comment on the ticket linking the PR and stating that review converged (e.g. `Review loop converged — PR <url> ready for review.`).
- Any failure here (no ticket, no MCP, API error) is logged and never fails the loop.

### Step 3: Report

After the loop exits (converged, no-progress, or at cap), emit a compact summary:

```text
review-loop on PR #<n> (<RESOLVED_REPO>)

Iterations: <N> of <CAP>
Stop reason: <all-threads-resolved | no-progress (flags remain) | cap-reached | ci-red (cap on ci-auto-fix handoffs) | ci-error (check query failed) | poll error | report-only (--no-feedback) | skipped (sub-agent dispatch unavailable) | skipped (nested dispatch — must run at top level)>
# Report the STOP_REASON the loop actually set — never re-derive it from the
# iteration count. `Iterations: 1 of 1` is what report-only, a first-iteration
# convergence, and a CAP=1 run all look like from the outside.
# The two skipped tokens are distinct on purpose. A nested dispatch is a caller
# bug with a same-day fix; an absent dispatch tool is the environment. Never report a
# skip as "report-only" because it is the nearest token — report-only means a
# review pass ran and its findings were not applied, which is the opposite of a
# PR that was never reviewed.
Review source: <pr-reviewer | external>
# No count on the external arm: the shared poll is a liveness probe and returns only
# NEW_FEEDBACK / NO_FEEDBACK / POLL_ERROR. It exposes no event count, and widening a
# shared contract with two callers for a report cosmetic is not worth it.

Per-iteration summary:
  Iteration 1: <verdict>, <N findings>, <M applied>, <A answered/resolved>, <K simplify recipes>, <U threads still open>
  Iteration 2: ...

Open threads at exit: <count>
  - <one line per still-open human-judgment flag / unresolved blocker>

CI at exit: <green | pending | red (<failing check names>) | error (<verbatim query failure>) | not run (--no-ci) | none on this repo>
  ci-auto-fix handoffs: <CI_HANDOFFS> of 2

Preview spec: <green (<N> specs on <url>) | red (<N> failing on <url>) — review before undrafting | inconclusive (preview not deployed at exit) | inconclusive (no deployment lookup on this access path) | inconclusive (<reason> at exit) | not run (no preview-spec block) | not run (empty preview-spec block) | not run (<reason>) | not run (unrecognised outcome: <verbatim>) | skipped (--no-preview-run) | skipped (--no-feedback) | skipped (preview-spec not available)>

PR description: <refreshed | unchanged (no code applied) | skipped (--no-refresh)>
Linear note: <posted <ticket> | no ticket linked | Linear MCP unavailable | skipped>

Final pr-reviewer verdict: <PASS | FAIL | n/a (external review)>
Head commit: <sha>
```

Surface remaining open threads prominently if the cap was reached or the
no-progress guard tripped. Do not silently drop them — an open thread at exit is
a human-judgment flag the user must resolve.

**A red check at exit gets the same treatment.** Name the failing checks and say
the loop stopped with CI red. Never describe such a run as converged — zero open
threads over a red build is not a review-ready PR.

## Hard rules

- **The only permitted `polish` invocation is `Skill("polish", "simplify")`.** Non-simplify modes trigger an internal agent pass and create a dispatch cycle.
- **This loop runs at the top level, never inside a sub-agent.** Its first sub-step is a delegation, so a caller that dispatches the loop instead of running it spends the delegation budget one level too high and the loop can only skip at iteration 0 ([Caller contract](#caller-contract--run-this-loop-at-the-top-level-never-in

…(truncated)