Flash is the default model; --m3 falls back to M3, --effort overrides. Defaults: max (review-minimax, review-loop, mm-worktree-task), medium (explore), low (ci-triage). ask-minimax-run applies MINIMAX_EFFORT to Flash models when --effort is not passed. Bump stale fixture date.
16 KiB
review-loop-run
Purpose
Launch a detached MiniMax M3 review of a review-loop artefact (plan, spec diff, test files, code diff, or a "is this bug still real" verification) with a kind-specific prompt built from a fixed template, and report its status/severity counts — so the /review-loop skill never hand-writes prompt files, launcher scripts, or pkills.
Usage
review-loop-run launch --kind <plan|spec|tests|code|verify> --name <tag> [--round N] \
--file <path> [--file <path> ...] [--context "<text>"] [--prev <result.md>] \
[--tools "<extra allowedTools>"] [--timeout <secs>] [--m3] [--effort <level>] [--dryrun|-n]
review-loop-run status [--name <tag>] [--round N]
review-loop-run counts <result.md>
review-loop-run --help
Subcommands
| Subcommand | Description |
|---|---|
launch |
Build the prompt, write it under the report dir, start ask-minimax-run detached via setsid nohup, print the prompt/stdout/result paths, exit 0 immediately. |
status |
For every <tag>-r<N> in the report dir (or the one named), print running/done exit=N, the result file's existence, and the CRITICAL/HIGH/MEDIUM/LOW counts parsed from it. Exit 0. |
counts |
Print CRITICAL=<n> HIGH=<n> MEDIUM=<n> LOW=<n> for one result file. Exit 0 when CRITICAL+HIGH == 0, else exit 4. |
Options (launch)
| Flag | Default | Description |
|---|---|---|
--kind |
required | Selects the prompt template and default best-practices set (§Templates). |
--name |
required | Tag for this artefact (e.g. spec-BC, code-D, verify-02a8436d). |
--round |
1 | Round number; appended to file names as -r<N>. Rounds ≥2 automatically add the previous round's result file (<tag>-r<N-1>-result.md, if present) to the reading list and a "do not re-raise adjudicated findings" instruction. |
--file |
≥1 required | Files the reviewer must read fully (diff files, test files, specs, item docs). Repeatable. |
--context |
— | One paragraph of task context (what the artefact closes, relevant source paths). |
--prev |
auto | Explicit previous-round result file (overrides the auto lookup). |
--tools |
kind default | Extra --allowedTools entries (comma-separated). |
--timeout |
1500 | Seconds before the sub-session is killed. |
--m3 |
off | Use MiniMax-M3 instead of the default MiniMax-M3.1-Flash-Preview (1M window natively, always thinks). No effort is sent unless --effort is given. |
--effort <level> |
max |
Thinking depth low|medium|high|xhigh|max, passed to ask-minimax-run --effort. Any other value exits 1. |
--project |
current repo root | Repo to cd into before launching. |
--dryrun, -n |
off | Print the assembled prompt and the exact launch command; write nothing; launch nothing. |
Environment
| Var | Effect |
|---|---|
REVIEW_LOOP_RUN_DIR |
Report dir root. Default ${XDG_RUNTIME_DIR:-/tmp}/review-loop/<repo-basename>. |
REVIEW_LOOP_BP_DIR |
Best-practices repo. Default ~/dev/claude/projects/best-practices. |
Behaviour
- Resolve the repo root (
git rev-parse --show-toplevel, or--project); derive the report dir;mkdir -p. - Validate: kind in the closed set, name matches
^[A-Za-z0-9._-]+$, every--fileexists,ask-minimax-runon PATH, MiniMax profile present. - Assemble the prompt from the kind template (§Templates): header (non-interactive, write to the output file), task text,
--context, the "Files to read" list =--fileentries + the kind's default best-practices/gotcha files that exist + the previous-round result (rounds ≥2), the output path, constraints. - Write
<dir>/<name>-r<N>-prompt.txt. - Write
<dir>/<name>-r<N>-launch.sh(a tiny bash script thatcds to the repo, runstimeout <secs> ask-minimax-run --model "$MODEL" --effort "$EFFORT" --allowedTools "<list>" --permission-mode acceptEdits -p "$(cat prompt)" > stdout 2>&1, then appendsexit=$?to the stdout file) and starts it withsetsid nohup <launch.sh> >/dev/null 2>&1 &. - Print the three paths and
launched.
status scans <dir>/*-r*-stdout.txt; a file whose last line matches ^exit= is done, else running; counts come from counts.
counts parses the review's own count-by-severity block first, never bare numbers scattered through the file (a finding heading like "## Finding 3 — HTTP 404 on ..." or "### MEDIUM: ..." must not be misread as a count):
- Locate the scope. Find the last line matching
/count.*severity|severity.*count|summary/iand take from that heading up to the next#-heading of the same or shallower depth (or EOF). If no such heading exists, the scope is the last 40 lines of the file. CR-10 (bug 8c865eb4): a deeper sub-heading (e.g.###under a##section) is part of the section and must NOT terminate the scope — a count block written as## Count by Severityfollowed by### CRITICAL: 0sub-headings keeps all four severity counts in scope. - Extract per severity, within scope only. For each of CRITICAL/HIGH/MEDIUM/LOW, take the last match in scope of any of the accepted forms below. A severity with no match in scope is 0 for this pass. Markdown emphasis markers around/inside the label are stripped before matching, so
**SEV:**also matches. The accepted forms are (bug 5352fe72 — Revokes prior text: the earlier form list required a bare-integer table cell and allowedSEV: nto match mid-prose; both are superseded here):- CR-7 Table row:
| SEV | n |or| SEV | n (<annotation>) |— the value cell may carry trailing text after the integer (e.g.| HIGH | 1 (H-R3-1: ...) |); the leading run of digits in the value cell is the count (thousands separators, decimals, and exponents are not interpreted —1,000reads as 1; counts are small integers in practice). A cell whose text does not begin with an integer does not match. - CR-8 Declaration line:
SEV: n/SEV = n/- SEV: n/**SEV**: n/SEV (n)/n SEV— accepted only when the line is a count declaration line: after stripping*emphasis markers and any leading list-bullet (-,*,+) or heading (#) prefix, the entire line consists of one or more such severity-count tokens separated only by spaces, commas, or semicolons, with nothing else. Prose lines that merely mention a count (e.g.MEDIUM=404 seen on 2026-09-13inside a quoted verdict) MUST NOT match — the trailing prose disqualifies the line. Exception: a short label parenthetical matching([A-Z][0-9]+)(e.g.- CRITICAL: 1 (C1)) is stripped before the declaration-only check and does not disqualify the line. Deliberately out of scope:_/__emphasis stripping, blockquote (>) and numbered-list prefixes, and any other parenthetical annotation on declaration lines (finding IDs like(H-R3-1)read as prose and are rejected; table cells are the first-class summary field).
- CR-7 Table row:
- Fallback (derived). Only if scope step 2 finds no severity value for any of CRITICAL/HIGH/MEDIUM/LOW (found_any == 0), fall back to counting lines that START with the severity word —
^\s*(#+\s*)?(\*\*)?(CRITICAL|HIGH|MEDIUM|LOW)\bfollowed by:,—, or-— one count per matching line, never bare numbers elsewhere. Append a literal(derived)marker to the output in this case. - CR-9 Suspect derived zero (bug 5352fe72). Applies only when step 3 actually ran (found_any == 0; an explicit count block yielding C=0 H=0 is trusted and never suspect). If the derived fallback yields CRITICAL=0 and HIGH=0 but the file contains a heading line matching
^#{1,6}\s*(\*\*)?(CRITICAL|HIGH)\b(case-insensitive) anywhere in the file — not restricted to the count-block scope — (a severity section header the derived regex cannot count, e.g. bare## CRITICAL), the result is untrustworthy: append(suspect)instead of(derived)to the output and exit 4 (fail closed) regardless of the printed counts. Fenced code blocks are NOT excluded from this scan: a false suspect on a file quoting## CRITICALin a fence is fail-closed (a human reads the file), which is the preferred failure direction. Suspect detection is restricted to CRITICAL/HIGH because only they gate the merge. Suspect and genuine C+H>0 share exit 4 deliberately — the consumer action (another round / human read) is identical; distinguish via the(suspect)marker. - Missing severity → 0. An empty result file prints
CRITICAL=0 HIGH=0 MEDIUM=0 LOW=0and exits 0. Output staysCRITICAL=n HIGH=n MEDIUM=n LOW=n(plus the optional trailing(derived)or(suspect)marker); exit 4 iff CRITICAL+HIGH>0 or the result is suspect, else 0.
Why (CR-7/CR-8): review models routinely annotate summary-table cells with finding IDs, and verify verdicts quote historic counts in prose; both produced silent 0/0 mis-reports on 2026-09-16 that defeated the /review-loop merge gate. Why (CR-9): a derived 0/0 on a file that visibly has CRITICAL/HIGH section headers means the parser failed, not that the review is clean — the gate must fail closed. Why (CR-10): a count block written as an h2 with h3 severity subsections (a common pattern in ## Count by Severity followed by ### CRITICAL: 0) was being mis-scope-terminated at the first h3, leaving scope empty, firing the derived fallback, and counting the ### CRITICAL: 0 heading itself as a CRITICAL finding — defeating the merge gate on genuinely clean reviews (bug 8c865eb4).
Fixtures: tests/fixtures/review-loop-run/{bullet-list,table,inline-bold,heading-numbers,no-count-block,annotated-table-cell,prose-historic-quote,suspect-derived,h3-subsections,h2-terminates-scope}.md cover, respectively: a bullet-list count block, a markdown-table count block, an inline **HIGH**: 2 style block, a file with "HTTP 404" and a bare number in headings alongside a correct trailing count-by-severity table (regression case for the 2026-09-13 MEDIUM=404/HIGH=11 mis-parse), and a file with no count block at all (derived fallback from finding headings) — plus the CR-7/CR-8/CR-9 fixtures — plus the CR-10 fixtures (h3-subsections.md: count block at ## with each severity value at ###; h2-terminates-scope.md: count block at ## with a real ## after it that must terminate scope and exclude a stray ### HIGH: 99 underneath).
Scenarios / test matrix (bug 5352fe72; test handles in tests/test-review-loop-run.sh):
| Req | Test handle | Fixture (required shape) | Expected stdout | Exit |
|---|---|---|---|---|
| CR-7 | counts annotated table cell |
annotated-table-cell.md: ## Count by severity then a table with rows | CRITICAL | 0 |, | HIGH | 1 (H-R3-1: gate bypass) |, | MEDIUM | 2 (M-1, M-2) |, | LOW | 0 | |
CRITICAL=0 HIGH=1 MEDIUM=2 LOW=0 |
— |
| CR-7 | counts annotated cell exits 4 |
same fixture | — | 4 |
| CR-8 | counts prose historic quote ignored |
prose-historic-quote.md: ## Count by severity then | HIGH | 2 | table rows (all other sevs 0) followed by prose line Note: the earlier bug reported MEDIUM=404 seen on 2026-09-13. inside the same scope |
CRITICAL=0 HIGH=2 MEDIUM=0 LOW=0 (MEDIUM must not be 404) |
4 |
| CR-9 | counts suspect derived zero marks suspect |
suspect-derived.md: no count block, a bare ## CRITICAL header with prose under it, no line matching the derived finding-line regex |
contains (suspect), not (derived) |
— |
| CR-9 | counts suspect derived zero exits 4 |
same fixture | — | 4 |
| CR-9 | counts suspect derived zero case insensitive |
suspect-derived-lower.md: same shape as suspect-derived.md but header is ## critical (lowercase) |
contains (suspect) |
4 |
| CR-8 | counts declaration line annotation rejected |
declaration-with-annotation.md: ## Count by severity then no table, only the bullet line - HIGH: 1 (H-R3-1) and no other severity lines |
HIGH must not be 1 (the parenthetical disqualifies the declaration line; the file then has no count values, so derived/suspect rules apply to whatever remains) | — |
| CR-10 | counts h3 subsections parse as zero |
h3-subsections.md: ## Count by Severity (h2) followed by ### CRITICAL: 0, ### HIGH: 0, ### MEDIUM: 0, ### LOW: 0 (all h3) — no table, no body declarations |
CRITICAL=0 HIGH=0 MEDIUM=0 LOW=0 |
0 |
| CR-10 | counts h2 still terminates scope |
h2-terminates-scope.md: ## Count by Severity (h2) with a trailing table | CRITICAL | 0 | ... | LOW | 0 |, then a ## Next Section (h2, same depth) and under it ### HIGH: 99 |
CRITICAL=0 HIGH=1 MEDIUM=2 LOW=0 (the ### HIGH: 99 under the second ## must be ignored — only the table counts) |
4 |
Templates (per --kind)
All prompts share: severity scheme CRITICAL/HIGH/MEDIUM/LOW, file:line citations, a concrete fix per finding, "no praise", a count-by-severity table at the end, "read every listed file fully; do not modify repository files".
| kind | task text | default reading set | allowlist |
|---|---|---|---|
verify |
"Is the planner item in the first file still an open defect on main? Verdict RESOLVED / STILL OPEN / PARTIAL / CANNOT DETERMINE with cited evidence; remaining work." | memory/gotchas-*.md not needed; adds git log access |
Read,Write,Glob,Grep,Bash(git log *),Bash(git show *),Bash(git grep *),Bash(git diff *) |
plan |
adversarial plan critique: missing requirements, unsafe sequencing, wrong code assumptions (verify), untestable statements, security gaps, prior gotchas | spec-driven-development, test-driven-development, security-architecture, api-design, validation, kubernetes + project memory/gotchas-*.md, memory/process-lessons.md |
Read,Write,Glob,Grep,Bash(git log *),Bash(git grep *) |
spec |
spec amendment review: ambiguity, untestable statements, contradictions with existing requirements, non-derivable test handles, ID collisions, security; the "symbol/module does not exist yet" class is out of scope for spec-ahead specs | spec-driven-development, test-driven-development, mechanical-test-generation, api-design, security-architecture, gotchas-workflow, gotchas-specs, gotchas-python | same as plan |
tests |
spec-ahead test review: every handle present and asserting normative behaviour, fixtures match pinned shapes, assertions would pass against a correct implementation, mock hygiene, fixtures that skip __init__, xfail reasons |
test-driven-development, mechanical-test-generation, gotchas-python | plan set + Bash(python3 -m pytest *) |
code |
implementation review against normative spec text and the tests: correctness, security, gotchas-python, behaviour changes to existing call sites, wire-shape changes, the branch must not touch tests/ or spec/ | security-architecture, llm-code-security, python-patterns, gotchas-python | plan set + Bash(git diff *) |
Dryrun behaviour
--dryrun prints: resolved repo and report dir, the reading list (marking any default file that is missing as (absent, skipped)), the full prompt text, the allowlist, the exact launch command line, and [dryrun] nothing launched. It creates no files.
Edge cases
- Unknown
--kind→ exit 2 with the closed set listed. - A
--filethat does not exist → exit 2 naming it (before anything is written). - Round ≥2 with no previous result → warning, proceeds without the "adjudicated" paragraph.
ask-minimax-runmissing or profile absent → exit 1.statuswith an empty report dir → printsno runsand exits 0.- Never uses
pkill; the launch script is self-contained so the user can kill by PID.
Examples
review-loop-run launch --kind spec --name spec-BC --round 2 \
--file /tmp/x/spec-BC.diff --context "WIA-17c..f close bug 02a8436d; source: controlplane/api/work_items.py"
review-loop-run status --name spec-BC
review-loop-run counts /run/user/1000/review-loop/agent-runtimes/spec-BC-r2-result.md