feat: review-loop-run — detached MiniMax review launcher for /review-loop artefacts
launch (per-kind prompt templates: verify/plan/spec/tests/code; best-practice + gotcha reading sets; setsid nohup launcher; round N+1 feeds back round N), status, counts (exit 4 on CRITICAL/HIGH). Spec + 29 dryrun tests. Claude-Session: https://claude.ai/code/session_018LRh1S2bHJYhEgrMu3UJx8
This commit is contained in:
Paul O'Reilly
260
scripts/review-loop-run
Executable file
260
scripts/review-loop-run
Executable file
@@ -0,0 +1,260 @@
|
||||
#!/usr/bin/env bash
|
||||
# review-loop-run — launch/track detached MiniMax reviews of review-loop artefacts.
|
||||
# Spec: specs/review-loop-run.spec.md
|
||||
set -euo pipefail
|
||||
|
||||
BLUE='\033[0;34m'; GREEN='\033[0;32m'; YELLOW='\033[1;33m'; RED='\033[0;31m'; NC='\033[0m'
|
||||
info() { printf "${BLUE}==>${NC} %s\n" "$*"; }
|
||||
warn() { printf "${YELLOW}warning:${NC} %s\n" "$*" >&2; }
|
||||
die() { printf "${RED}error:${NC} %s\n" "$*" >&2; exit "${2:-1}"; }
|
||||
|
||||
BP_DIR="${REVIEW_LOOP_BP_DIR:-$HOME/dev/claude/projects/best-practices}"
|
||||
KINDS="plan spec tests code verify"
|
||||
|
||||
usage() {
|
||||
cat <<'EOF'
|
||||
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>"] [--timeout <secs>]
|
||||
[--project <repo>] [--dryrun|-n]
|
||||
review-loop-run status [--name <tag>] [--round N] [--project <repo>]
|
||||
review-loop-run counts <result.md>
|
||||
review-loop-run --help
|
||||
|
||||
Launches ask-minimax-run detached (setsid nohup) with a kind-specific review
|
||||
prompt; files land in ${XDG_RUNTIME_DIR:-/tmp}/review-loop/<repo>/<tag>-r<N>-{prompt.txt,launch.sh,stdout.txt,result.md}.
|
||||
counts exits 4 when CRITICAL+HIGH > 0.
|
||||
EOF
|
||||
}
|
||||
|
||||
resolve_repo() {
|
||||
local p="${1:-}"
|
||||
if [[ -n "$p" ]]; then cd "$p"; fi
|
||||
git rev-parse --show-toplevel 2>/dev/null || pwd
|
||||
}
|
||||
|
||||
report_dir_for() {
|
||||
local repo="$1"
|
||||
echo "${REVIEW_LOOP_RUN_DIR:-${XDG_RUNTIME_DIR:-/tmp}/review-loop/$(basename "$repo")}"
|
||||
}
|
||||
|
||||
# counts <file> → prints CRITICAL=n HIGH=n MEDIUM=n LOW=n ; exit 4 if C+H>0
|
||||
counts() {
|
||||
local f="$1"
|
||||
[[ -f "$f" ]] || die "result file not found: $f" 2
|
||||
local out="" total=0
|
||||
for sev in CRITICAL HIGH MEDIUM LOW; do
|
||||
local n
|
||||
n=$(grep -iE "^\| *${sev} *\| *[0-9]+" "$f" | head -1 | sed -E 's/^\| *[A-Za-z]+ *\| *([0-9]+).*/\1/') || true
|
||||
if [[ -z "$n" ]]; then
|
||||
n=$(grep -iE "^[-*]* *\**${sev}:?\**:? *\**([0-9]+)" "$f" | head -1 | sed -E 's/.*[^0-9]([0-9]+)[^0-9]*$/\1/') || true
|
||||
fi
|
||||
if [[ -z "$n" ]]; then
|
||||
n=$(grep -cE "^#{2,4} .*\b${sev}\b" "$f") || true
|
||||
fi
|
||||
n="${n:-0}"; out+="${sev}=${n} "
|
||||
if [[ "$sev" == "CRITICAL" || "$sev" == "HIGH" ]]; then total=$((total + n)); fi
|
||||
done
|
||||
echo "$out"
|
||||
if [[ "$total" -gt 0 ]]; then return 4; fi
|
||||
return 0
|
||||
}
|
||||
|
||||
kind_task() {
|
||||
case "$1" in
|
||||
verify) cat <<'EOF'
|
||||
Verify whether the planner work item described in the first listed file is STILL an open, unresolved issue in the current code on main, or whether it has already been fixed/implemented and the record is merely stale. Extract the concrete claim, inspect the current source with Grep/Glob/Read and `git log -S`/`git show`, and decide from the CODE, not the item's own text.
|
||||
|
||||
Output format:
|
||||
- **Item:** <uuid> — <title>
|
||||
- **Verdict:** RESOLVED / STILL OPEN / PARTIALLY RESOLVED / CANNOT DETERMINE
|
||||
- **Evidence:** 3-8 bullets, each citing file:line or a commit sha
|
||||
- **Remaining work (if any):** concise
|
||||
- **Confidence:** high/medium/low
|
||||
EOF
|
||||
;;
|
||||
plan) cat <<'EOF'
|
||||
Independent, adversarial critical review of a program/implementation plan. For every finding give: severity (CRITICAL/HIGH/MEDIUM/LOW), the plan section, what is wrong, which best-practice/gotcha it violates (file + heading), and a concrete fix. Look for missing requirements, unsafe sequencing, wrong assumptions about the code (verify against source with Grep/Read), untestable statements, security gaps, ID collisions with existing requirement IDs, and anything the gotcha files say has bitten before. Verify every cited file:line and requirement ID exists. Do not pad with praise. End with a count by severity.
|
||||
EOF
|
||||
;;
|
||||
spec) cat <<'EOF'
|
||||
Independent critical spec review. The amendment under review is the unified diff (or spec section) in the listed file(s); the working tree already contains the amended text. Check: ambiguity; untestable statements; missing error/edge cases; contradictions with existing requirements in the same spec; test handles that cannot be derived mechanically (mechanical-test-generation.md); requirement-ID collisions; security gaps; cross-module seams pinned to real signatures and attribute names (verify against the source). OUT OF SCOPE: reporting that a symbol/module/field named by the spec "does not exist yet" — this is a spec-ahead artefact and new symbols are marked NEW in its module-layout rows. For each finding: severity, requirement ID, what is wrong, the best-practice/gotcha violated (file + heading), concrete fix. No praise. End with a count by severity.
|
||||
EOF
|
||||
;;
|
||||
tests) cat <<'EOF'
|
||||
Independent critical review of spec-ahead TEST files. Tests for not-yet-implemented behaviour are @pytest.mark.xfail(strict=True); that is intended. Check: (1) every spec test handle exists with the exact name and asserts the requirement's normative behaviour (not merely something already true today); (2) fixtures match the spec's pinned shapes; (3) assertions would pass against a CORRECT implementation — flag wrong attribute names, wrong status codes/envelopes, mocking the wrong target, invented private helper names, fixtures built with object.__new__ that skip __init__ attributes, assertions that xfail today for an unrelated reason (permanent false negatives); (4) test isolation and mock hygiene per test-driven-development.md and gotchas-python.md (AsyncMock on sync calls, TestClient lifespan, shared state); (5) xfail reasons name the right requirement. Severity CRITICAL/HIGH/MEDIUM/LOW; cite file:line; concrete fix per finding; no praise; end with a count by severity.
|
||||
EOF
|
||||
;;
|
||||
code) cat <<'EOF'
|
||||
Independent critical CODE review of an implementation branch. The diff is in the listed diff file (branch vs main); read source in the worktree/branch named in the context, not the main checkout. Review the implementation against the normative spec text and the tests (the acceptance criteria; the implementer was forbidden from editing tests/ or spec/ — confirm the diff touches neither). Check: (1) every requirement clause is implemented as written (cite spec sentence vs code); (2) correctness bugs, edge cases, fail-open paths, exception swallowing, async misuse, mutation of shared state, ordering relative to the application lifespan (memoised state created before its dependencies are wired); (3) security (input validation, RBAC/identity handling, error leakage per security-architecture.md and llm-code-security.md); (4) gotchas-python.md patterns (Pydantic extra=ignore drops, mock-target divergence, logging reserved attrs, sync/async); (5) unintended behaviour changes to existing call sites and any wire-shape change (request/response fields, error codes, log formats); (6) code quality: duplication, dead code, docstrings referencing spec IDs. Severity CRITICAL/HIGH/MEDIUM/LOW; cite file:line; concrete fix per finding; no praise; end with a count by severity.
|
||||
EOF
|
||||
;;
|
||||
esac
|
||||
}
|
||||
|
||||
kind_defaults() {
|
||||
local repo="$1"
|
||||
case "$2" in
|
||||
verify) : ;;
|
||||
plan) printf '%s\n' "$BP_DIR/spec-driven-development.md" "$BP_DIR/test-driven-development.md" "$BP_DIR/security-architecture.md" "$BP_DIR/api-design.md" "$BP_DIR/validation.md" "$BP_DIR/kubernetes.md" "$repo"/memory/gotchas-*.md "$repo/memory/process-lessons.md" ;;
|
||||
spec) printf '%s\n' "$BP_DIR/spec-driven-development.md" "$BP_DIR/test-driven-development.md" "$BP_DIR/mechanical-test-generation.md" "$BP_DIR/api-design.md" "$BP_DIR/security-architecture.md" "$repo/memory/gotchas-workflow.md" "$repo/memory/gotchas-specs.md" "$repo/memory/gotchas-python.md" ;;
|
||||
tests) printf '%s\n' "$BP_DIR/test-driven-development.md" "$BP_DIR/mechanical-test-generation.md" "$repo/memory/gotchas-python.md" ;;
|
||||
code) printf '%s\n' "$BP_DIR/security-architecture.md" "$BP_DIR/llm-code-security.md" "$BP_DIR/python-patterns.md" "$repo/memory/gotchas-python.md" ;;
|
||||
esac
|
||||
}
|
||||
|
||||
kind_tools() {
|
||||
local base="Read,Write,Glob,Grep,Bash(git log *),Bash(git grep *)"
|
||||
case "$1" in
|
||||
verify) echo "Read,Write,Glob,Grep,Bash(git log *),Bash(git show *),Bash(git grep *),Bash(git diff *)" ;;
|
||||
tests) echo "$base,Bash(python3 -m pytest *)" ;;
|
||||
code) echo "$base,Bash(git diff *)" ;;
|
||||
*) echo "$base" ;;
|
||||
esac
|
||||
}
|
||||
|
||||
cmd_launch() {
|
||||
local kind="" name="" round=1 context="" prev="" tools="" timeout=1500 project="" dryrun=0
|
||||
local -a files=()
|
||||
while [[ $# -gt 0 ]]; do
|
||||
case "$1" in
|
||||
--kind) kind="$2"; shift 2 ;;
|
||||
--name) name="$2"; shift 2 ;;
|
||||
--round) round="$2"; shift 2 ;;
|
||||
--file) files+=("$2"); shift 2 ;;
|
||||
--context) context="$2"; shift 2 ;;
|
||||
--prev) prev="$2"; shift 2 ;;
|
||||
--tools) tools="$2"; shift 2 ;;
|
||||
--timeout) timeout="$2"; shift 2 ;;
|
||||
--project) project="$2"; shift 2 ;;
|
||||
--dryrun|-n) dryrun=1; shift ;;
|
||||
-h|--help) usage; exit 0 ;;
|
||||
*) die "unknown option: $1" 2 ;;
|
||||
esac
|
||||
done
|
||||
[[ " $KINDS " == *" $kind "* ]] || die "--kind must be one of: $KINDS" 2
|
||||
[[ "$name" =~ ^[A-Za-z0-9._-]+$ ]] || die "--name must match ^[A-Za-z0-9._-]+\$" 2
|
||||
[[ "$round" =~ ^[0-9]+$ ]] || die "--round must be an integer" 2
|
||||
[[ ${#files[@]} -ge 1 ]] || die "at least one --file is required" 2
|
||||
for f in "${files[@]}"; do [[ -f "$f" ]] || die "--file not found: $f" 2; done
|
||||
command -v ask-minimax-run >/dev/null || die "ask-minimax-run not on PATH"
|
||||
[[ -f "$HOME/.claude-oreillyit-minimax/provider.env" ]] || die "MiniMax profile missing (~/.claude-oreillyit-minimax/provider.env)"
|
||||
|
||||
local repo dir tag
|
||||
repo=$(resolve_repo "$project"); dir=$(report_dir_for "$repo"); tag="${name}-r${round}"
|
||||
local prompt="$dir/$tag-prompt.txt" launch="$dir/$tag-launch.sh" stdout="$dir/$tag-stdout.txt" result="$dir/$tag-result.md"
|
||||
|
||||
# previous round
|
||||
local prev_para=""
|
||||
if [[ -z "$prev" && "$round" -ge 2 ]]; then
|
||||
local cand="$dir/${name}-r$((round-1))-result.md"
|
||||
[[ -f "$cand" ]] && prev="$cand"
|
||||
fi
|
||||
if [[ "$round" -ge 2 ]]; then
|
||||
if [[ -n "$prev" && -f "$prev" ]]; then
|
||||
prev_para="This is ROUND ${round}. The previous round's findings are in ${prev} and were adjudicated by the supervisor (accepted findings were applied; rejected findings carry a source-cited reason on the artefact). Read that file; do not re-raise resolved or rejected findings unless you have NEW evidence; report only CRITICAL/HIGH plus any MEDIUM that blocks mechanical test derivation."
|
||||
else
|
||||
warn "round $round but no previous result found; proceeding without the adjudication paragraph"
|
||||
fi
|
||||
fi
|
||||
|
||||
# reading list
|
||||
local -a reading=("${files[@]}")
|
||||
[[ -n "$prev" && -f "$prev" ]] && reading+=("$prev")
|
||||
local -a defaults=() missing=()
|
||||
while IFS= read -r d; do
|
||||
[[ -z "$d" ]] && continue
|
||||
if [[ -f "$d" ]]; then defaults+=("$d"); else missing+=("$d"); fi
|
||||
done < <(kind_defaults "$repo" "$kind")
|
||||
reading+=("${defaults[@]}")
|
||||
|
||||
local allow; allow=$(kind_tools "$kind"); [[ -n "$tools" ]] && allow="$allow,$tools"
|
||||
|
||||
local body
|
||||
body=$(cat <<EOF
|
||||
You are running in non-interactive (-p) mode. Complete the task and write your full response to the output file below using the Write tool. Do not rely on stdout for structured output.
|
||||
|
||||
## Task
|
||||
$(kind_task "$kind")
|
||||
${context:+
|
||||
Context: $context}
|
||||
${prev_para:+
|
||||
$prev_para}
|
||||
|
||||
## Files to read
|
||||
$(printf '%s\n' "${reading[@]}")
|
||||
|
||||
## Output file
|
||||
Write to: $result
|
||||
|
||||
Use the Write tool to create that file. Overwrite if it already exists.
|
||||
|
||||
## Constraints
|
||||
- Read every listed file fully before writing.
|
||||
- Cite file:line or commit sha for every claim.
|
||||
- Do not modify any repository files. Only write the output file.
|
||||
EOF
|
||||
)
|
||||
local launch_body
|
||||
launch_body=$(cat <<EOF
|
||||
#!/usr/bin/env bash
|
||||
cd "$repo" || exit 1
|
||||
timeout ${timeout} ask-minimax-run --model MiniMax-M3 --allowedTools "$allow" --permission-mode acceptEdits -p "\$(cat "$prompt")" > "$stdout" 2>&1
|
||||
echo "exit=\$?" >> "$stdout"
|
||||
EOF
|
||||
)
|
||||
if [[ "$dryrun" -eq 1 ]]; then
|
||||
printf "${YELLOW}[dryrun]${NC} review-loop-run — nothing will be launched\n"
|
||||
echo " Repo: $repo"; echo " Report dir: $dir"; echo " Kind/round: $kind r$round"; echo " Allowlist: $allow"
|
||||
for m in "${missing[@]:-}"; do [[ -n "$m" ]] && echo " default file absent, skipped: $m"; done
|
||||
echo "--- prompt ---"; echo "$body"; echo "--- launch command ---"; echo "$launch_body"
|
||||
echo "[dryrun] nothing launched"; return 0
|
||||
fi
|
||||
mkdir -p "$dir"
|
||||
printf '%s\n' "$body" > "$prompt"
|
||||
printf '%s\n' "$launch_body" > "$launch"; chmod +x "$launch"
|
||||
rm -f "$stdout" "$result"
|
||||
setsid nohup "$launch" >/dev/null 2>&1 &
|
||||
info "launched $kind review $tag"
|
||||
echo " prompt: $prompt"; echo " stdout: $stdout"; echo " result: $result"
|
||||
echo " poll: review-loop-run status --name $name --round $round"
|
||||
}
|
||||
|
||||
cmd_status() {
|
||||
local name="" round="" project=""
|
||||
while [[ $# -gt 0 ]]; do
|
||||
case "$1" in
|
||||
--name) name="$2"; shift 2 ;;
|
||||
--round) round="$2"; shift 2 ;;
|
||||
--project) project="$2"; shift 2 ;;
|
||||
-h|--help) usage; exit 0 ;;
|
||||
*) die "unknown option: $1" 2 ;;
|
||||
esac
|
||||
done
|
||||
local repo dir; repo=$(resolve_repo "$project"); dir=$(report_dir_for "$repo")
|
||||
local pat="${name:-*}-r${round:-*}-stdout.txt"
|
||||
local any=0
|
||||
for so in "$dir"/$pat; do
|
||||
[[ -f "$so" ]] || continue
|
||||
any=1
|
||||
local tag="${so##*/}"; tag="${tag%-stdout.txt}"
|
||||
local res="$dir/$tag-result.md" state="running" c=""
|
||||
if tail -1 "$so" 2>/dev/null | grep -q '^exit='; then state="done $(tail -1 "$so")"; fi
|
||||
if [[ -f "$res" ]]; then c=$(counts "$res" 2>/dev/null || true); [[ -z "$c" ]] && c=$(counts "$res" 2>/dev/null; true); else c="(no result yet)"; fi
|
||||
printf "%-28s %-14s %s\n" "$tag" "$state" "$c"
|
||||
done
|
||||
[[ "$any" -eq 1 ]] || echo "no runs in $dir"
|
||||
}
|
||||
|
||||
main() {
|
||||
[[ $# -ge 1 ]] || { usage; exit 2; }
|
||||
case "$1" in
|
||||
launch) shift; cmd_launch "$@" ;;
|
||||
status) shift; cmd_status "$@" ;;
|
||||
counts) shift; [[ $# -ge 1 ]] || die "counts <result.md>" 2; counts "$1" ;;
|
||||
-h|--help) usage ;;
|
||||
*) die "unknown subcommand: $1" 2 ;;
|
||||
esac
|
||||
}
|
||||
main "$@"
|
||||
Reference in New Issue
Block a user