# 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 `pkill`s.

## 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>] [--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. |
| `--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

1. Resolve the repo root (`git rev-parse --show-toplevel`, or `--project`); derive the report dir; `mkdir -p`.
2. Validate: kind in the closed set, name matches `^[A-Za-z0-9._-]+$`, every `--file` exists, `ask-minimax-run` on PATH, MiniMax profile present.
3. Assemble the prompt from the kind template (§Templates): header (non-interactive, write to the output file), task text, `--context`, the "Files to read" list = `--file` entries + the kind's default best-practices/gotcha files that exist + the previous-round result (rounds ≥2), the output path, constraints.
4. Write `<dir>/<name>-r<N>-prompt.txt`.
5. Write `<dir>/<name>-r<N>-launch.sh` (a tiny bash script that `cd`s to the repo, runs `timeout <secs> ask-minimax-run --model MiniMax-M3 --allowedTools "<list>" --permission-mode acceptEdits -p "$(cat prompt)" > stdout 2>&1`, then appends `exit=$?` to the stdout file) and starts it with `setsid nohup <launch.sh> >/dev/null 2>&1 &`.
6. 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):

1. **Locate the scope.** Find the *last* line matching `/count.*severity|severity.*count|summary/i` and take from that heading up to the next `#`-heading (or EOF). If no such heading exists, the scope is the last 40 lines of the file.
2. **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 allowed `SEV: n` to 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,000` reads 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-13` inside 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).
3. **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)\b` followed by `:`, `—`, or `-` — one count per matching line, never bare numbers elsewhere. Append a literal ` (derived)` marker to the output in this case.
4. **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 `## CRITICAL` in 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.
5. Missing severity → 0. An empty result file prints `CRITICAL=0 HIGH=0 MEDIUM=0 LOW=0` and exits 0. Output stays `CRITICAL=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.

Fixtures: `tests/fixtures/review-loop-run/{bullet-list,table,inline-bold,heading-numbers,no-count-block,annotated-table-cell,prose-historic-quote,suspect-derived}.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 three new fixtures below.

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) | — |

## 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 `--file` that 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-run` missing or profile absent → exit 1.
- `status` with an empty report dir → prints `no runs` and 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
```