---
name: review-loop
description: >
  Supervised delivery loop for planner-backed projects: verify the work item is still
  real, then plan, spec, tests, code, integrate, deploy, with MiniMax reviewing every
  artefact against best-practices plus gotchas until no CRITICAL/HIGH, Sonnet writing
  the spec-ahead tests, MiniMax implementing in its own git worktree, and the supervisor
  gating that tests/ and spec/ are untouched before merging. Pass one or more work-item
  UUIDs (and optionally the entry stage) as arguments.
user_invocable: true
allowed-tools: Read, Write, Edit, Agent, SendMessage, Bash(date *), Bash(test *), Bash(ls *), Bash(cat *), Bash(mkdir *), Bash(chmod *), Bash(setsid *), Bash(pgrep *), Bash(python3 *), Bash(git status*), Bash(git rev-parse *), Bash(git log *), Bash(git diff *), Bash(git add *), Bash(git commit *), Bash(git pull *), Bash(git push), Bash(git merge *), Bash(git worktree *), Bash(git checkout main -- *), Bash(git -C *), Bash(mm-worktree-task *), Bash(review-loop-run *), Bash(review-minimax-run *), Bash(gac *), Bash(kubectl *), Bash(ssh-add *)
---

<command-name>review-loop</command-name>

# /review-loop Skill

Drive a planner work item (or a small batch) from "is this still a real issue?"
to "deployed and verified live" with an independent MiniMax review of every
artefact. The loop is what shipped WIA-17c..f and PDIV on 2026-09-13 (program
item `824222cb` in agent-runtimes holds every round as a worked example).

## Pre-gathered context

### Current date
!`date +%Y-%m-%d`

### Tooling (check via tool calls before step 0)
Run these with the Bash tool and stop if any is MISSING:
`test -f "$HOME/.claude-oreillyit-minimax/provider.env"` (MiniMax profile),
`test -x "$HOME/sbin/mm-worktree-task"`, and
`test -x "$HOME/sbin/review-loop-run"` (the review launcher; source in small-scripts).

### Planner + repo
!`git rev-parse --abbrev-ref HEAD`
!`git status --short | grep -v uv.lock | head -5`

## Arguments

`$ARGUMENTS` = one or more work-item UUIDs, optionally followed by `--from <stage>`
(`discover|plan|spec|tests|code|integrate|deploy`). Without `--from`, the skill
runs discovery first and decides the entry stage from what exists.

## Roles

| Role | Who | Never does |
|---|---|---|
| Supervisor | this session | write tests or code MiniMax should write; skip a gate |
| Verifier / reviewer | MiniMax M3.1-Flash via `review-loop-run launch` (detached) | modify repo files |
| Test author | Sonnet subagent (`Agent`, model sonnet) | touch spec/, controlplane/, lib/ |
| Coder | MiniMax via `mm-worktree-task` in its own git worktree | touch tests/ or spec/ |

Each work item gets its own worktree and branch (`mm-<slug>` / `mm/<slug>`);
nothing is developed on main. (The tooling uses git *worktrees*; a git subtree
is a different mechanism and is not used.)

## The loop

Every artefact goes: draft → MiniMax review → adjudicate (accept / reject with a
source-cited reason) → apply → re-review, until a round has zero CRITICAL and
zero HIGH. Cap 4 rounds; record residue on the item and move on. Reject up front
the "symbol/module does not exist yet" class for spec-ahead artefacts, or it
dominates every round.

Every review is launched with one command (prompt template, best-practice and
gotcha reading set, detached `setsid nohup` launch, and severity parsing are all
built in):
```
review-loop-run launch --kind <verify|plan|spec|tests|code> --name <tag> --round <N> \
  --file <artefact> [--file ...] --context "<one paragraph: what it closes, source paths>"
review-loop-run status --name <tag>        # running / done exit=N + severity counts
review-loop-run counts <result.md>         # exit 4 while CRITICAL+HIGH > 0 = another round
```
Round N+1 automatically feeds round N's result back with the "do not re-raise
adjudicated findings" instruction. Preview a prompt with `--dryrun`. Reviews take
5-10 minutes: schedule a wakeup instead of polling `status`.

### 0. Discover (per UUID)
1. `python3 -m lib.cp_cli planner items get <uuid>` + `documents list/get` into the scratchpad.
2. `review-loop-run launch --kind verify --name verify-<uuid8> --file <item.md> [--file <doc>...]`
   (verdict RESOLVED / STILL OPEN / PARTIAL with cited evidence). Spot-check one
   citation with git yourself.
3. RESOLVED → `planner items patch <uuid> --state completed --description "<evidence>"`.
   PARTIAL / OPEN → attach the verdict as `verification-<date>.md` and continue.

### 1. Plan (only for items that need a decision)
Write the plan to the scratchpad; upload as `plan-v1.md` on the item;
`review-loop-run launch --kind plan --name plan-<tag> --file <plan.md>` (the
kind's reading set covers spec-driven-development, test-driven-development,
security-architecture, api-design, validation, kubernetes and the project's
`memory/gotchas-*.md`). The plan MUST carry an ID-allocation table (every new
requirement ID checked against the spec files for collisions) and a
decision record `{options, choice, rationale, rejected, follow_ups}`.

### 2. Spec
Amend the owning `spec/*.md` in place (amend, don't add, when reversing prior
text; add `Revokes prior text:` notes). Every requirement gets: normative
statement, **Why**, **Test handles** with exact test names and file, and a
module-layout row for every new/changed file. Pin cross-module seams to exact
signatures and the real attribute names — read the source first; the reviewer
will catch invented names but only after a wasted round.
Write `git diff -- spec/<file>.md` to the scratchpad and run
`review-loop-run launch --kind spec --name spec-<tag> --file <diff> --context "..."`.
Commit `spec: <IDs> (<item uuids>)` when clean.

### 3. Tests (Sonnet)
One subagent per independent area. Brief: read the spec block + every cited
source file; create exactly the named handles in the named files; every new test
`@pytest.mark.xfail(strict=True, reason="<ID> not implemented")`; tests that
already pass today stay unmarked and are reported; access not-yet-existing
symbols inside the test body (never `from x import New` at top, never
`importorskip`); must not modify spec/ or source; run the files and the
`--co` of the whole suite. Gate: 0 xpassed, 0 errors. Then
`review-loop-run launch --kind tests --name tests-<tag> --file <spec diff> --file <test files...>`
(the template checks mock-target divergence, `object.__new__` fixtures that skip
`__init__`, `AsyncMock` on sync calls, assertions that pass today for the wrong
reason). Send accepted findings back to
the same Sonnet agent with `SendMessage`. Commit `test: … — xfail strict`.

### 4. Code (MiniMax in a worktree)
Create a child task item under the program item; attach `brief.md` (spec IDs,
the exact test files, the hard rules). Launch detached (max 2 concurrent — the
host runs CI builders):
```
mm-worktree-task <uuid> --docs all --timeout 3600 \
  --focus "Make the xfail(strict) tests in brief.md pass; never modify tests/ or spec/; run the named files with --runxfail and the whole test directory before reporting"
```
Collect with `mm-worktree-task <uuid> --collect`.
**Gates, in order:**
1. `git diff main...mm/<slug> -- tests/ spec/` is EMPTY. If not: `git checkout main -- tests/ spec/` in the worktree, note the violation, re-run.
2. In the worktree: target files with `--runxfail` all pass; the whole test directory has no new failures (strict XPASS of the new tests is the expected signal).
3. A test that cannot pass is a test or spec bug → fix it on main via the Sonnet agent, commit, `git -C <worktree> merge main`.
4. Write `git diff main...mm/<slug>` to the scratchpad; `review-loop-run launch --kind code --name code-<tag> --file <diff> --file <spec diff> --context "worktree: <path>; tests: <files>"` (correctness vs spec text, security, gotchas-python, wire-shape changes, no test edits). Small accepted fixes: apply yourself in the worktree and commit; large ones: re-run the coder with a `review-r1-fixes.md` brief.

### 5. Integrate
```
git worktree add -f /tmp/claude-1000/test-merge -b test-merge main
git -C /tmp/claude-1000/test-merge merge --no-edit mm/<slug>   # each branch
```
Strip the xfail markers there (per-test decorators, `_XFAIL` constants,
module-level `pytestmark`, `pytest.param(..., marks=...)` rows), run the FULL
suite, fast-forward main (`git merge --ff-only test-merge`), push, remove the
worktree. Regenerate `openapi/` + SDKs only if a route/schema changed (the local
generator produces version-noise churn otherwise — revert it).

### 6. Deploy and verify live
Empty commit `[CI BUILD] [DEPLOY] deploy: …`; watch with
`GITEA_TOKEN_FILE=~/dev/claude/secrets/gitea/ai_admin/token gac tail <owner/repo> --until run-complete`
(detached; a later push cancels an in-flight run — re-trigger). Then
`kubectl rollout status`, confirm the image tag, and **reproduce the original
bug's exact user-visible action** plus a smoke of unrelated reads. Only then
`planner items patch <uuid> --state completed --description "<evidence>"`.
If reads look wrong: roll back first (deploy-repo `newTag` + ArgoCD hard
refresh), diagnose second, file an incident item with an RCA.

## Bookkeeping
- Every review result → `documents put <program-item> <artefact>-review-r<N>.md`.
- Follow-ups discovered in review → new `future`/`task` items under the program item, never left in prose.
- End: `/log`, CONTEXT.md cache, commit + push both the project and any secondary repo touched (e.g. `agent-runtime-framework`).

## Hard rules and gotchas
- Launch every MiniMax review through `review-loop-run` (it uses a `setsid nohup` launcher script); never hand-roll `ask-minimax-run` launches and never `pkill -f` on the model name (it kills the invoking shell). To stop a run, kill the PID of its `<tag>-launch.sh`.
- Memoising helpers need an ordering test that drives the real lifespan — unit tests with a bare `app_state` cannot see `create_app()` vs lifespan ordering (incident aa98d9fe).
- "Verified live" means the original repro, not just a 200 on an unrelated route.
- Never `cd` into a worktree in a way that moves the session cwd; use `git -C` or `(cd … && …)`.
- The supervisor owns marker removal, merges, and pushes; agents never commit on main.