Files
custom-claude-skills/skills/review-loop/SKILL.md
Paul O'Reilly 06cc36ca7a minimax skills: move to MiniMax-M3.1-Flash-Preview with effort levels
ask-minimax* run at effort max; wrapper-based skills document their
default effort (explore medium, ci-triage low, review max).
2026-10-13 10:23:53 +13:00

10 KiB

name, description, user_invocable, allowed-tools
name description user_invocable allowed-tools
review-loop 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. true 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 *)

review-loop

/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.