Files
small-scripts/scripts/review-loop-run

374 lines
19 KiB
Bash
Executable File

#!/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_scope <file> → prints the text of the last "count by severity"
# section (heading matching /count.*severity|severity.*count|summary/i, up
# to the next heading or EOF), or the last 40 lines if no such heading exists.
_counts_scope() {
local f="$1"
local hdr_line
hdr_line=$(grep -inE 'count.*severity|severity.*count|summary' "$f" | tail -1 | cut -d: -f1) || true
if [[ -n "$hdr_line" ]]; then
awk -v start="$hdr_line" '
NR==start { found=1 }
found && NR>start && /^#{1,4}[[:space:]]/ { exit }
found { print }
' "$f"
else
tail -n 40 "$f"
fi
}
# _is_declaration_line <line> → 0 (true) iff the line is a declaration-only
# line. After stripping `*` emphasis markers, the line must consist solely of
# declaration tokens (CRITICAL/HIGH/MEDIUM/LOW, any severity) separated by
# [.,;\s]+, with nothing else except short label parentheticals matching
# [A-Z][0-9]+ (e.g. "(C1)"), which are stripped first (CR-8 label
# exception). Any other parenthetical or trailing prose (finding IDs
# like "(H-R3-1)", "(see §3.2)") disqualifies the line.
_is_declaration_line() {
local line="$1"
# Strip `*` emphasis markers
local clean="${line//\*/}"
# Reduce declaration tokens to placeholders (any severity)
local reduced
reduced=$(printf '%s\n' "$clean" | sed -E 's/(CRITICAL|HIGH|MEDIUM|LOW)[[:space:]]*[:=][[:space:]]*[0-9]+/__TOK__/g')
reduced=$(printf '%s\n' "$reduced" | sed -E 's/(CRITICAL|HIGH|MEDIUM|LOW)[[:space:]]*\([0-9]+\)/__TOK__/g')
reduced=$(printf '%s\n' "$reduced" | sed -E 's/[0-9]+[[:space:]]+(CRITICAL|HIGH|MEDIUM|LOW)/__TOK__/g')
# Strip short label parentheticals (single uppercase letter + digits)
reduced=$(printf '%s\n' "$reduced" | sed -E 's/[[:space:]]*\(([A-Z][0-9]+)\)//g')
# Remaining must be only separators + __TOK__s.
# Use a variable for the pattern to keep bash from mis-parsing the
# embedded `;` and `[[:space:]]` inside `[[ =~ ]]`.
local re='^[[:space:].;,]*__TOK__([.,;[:space:]]+__TOK__)*[[:space:].;,]*$'
[[ "$reduced" =~ $re ]]
}
# _counts_extract <scope-text> <sev> → last matching value for SEV in scope.
# CR-7: Table rows accept annotated value cells ("| SEV | n (...) |") — the
# leading integer in the value cell is the count.
# CR-8: Declaration forms ("SEV: n", "SEV = n", "SEV (n)", "n SEV", and their
# bulleted/bold variants) match only on declaration-only lines — a line
# that, after stripping `*` and any leading bullet/heading prefix,
# consists solely of declaration tokens separated by [.,;\s]+ (label-
# style parentheticals like "(C1)" are accepted; other parentheticals
# disqualify the line).
_counts_extract() {
local scope="$1" sev="$2"
# CR-7: Table row branch (exempt from declaration-only rule).
# Accept "| SEV | n |" and "| SEV | n (<annotation>) |".
local table_match
table_match=$(grep -inE "\\|[[:space:]]*${sev}[[:space:]]*\\|[[:space:]]*[0-9]+[^|]*\\|" <<<"$scope" | tail -1) || true
if [[ -n "$table_match" ]]; then
if [[ "$table_match" =~ \|[[:space:]]*${sev}[[:space:]]*\|[[:space:]]*([0-9]+)[^|]*\| ]]; then
echo "${BASH_REMATCH[1]}"
return
fi
fi
# CR-8: Declaration line branch — only lines that pass _is_declaration_line
# contribute; across all such lines the LAST match wins.
local last_n=""
while IFS= read -r line; do
[[ -z "$line" ]] && continue
# Strip leading bullet (-, *, +) or heading (#+) prefix
local stripped
stripped=$(printf '%s\n' "$line" | sed -E 's/^[[:space:]]*([-*+]+|#{1,6})[[:space:]]*//')
# Reject the line if it's not declaration-only
_is_declaration_line "$stripped" || continue
# Strip `*` markers for value extraction
local clean_line="${stripped//\*/}"
local n=""
if [[ "$clean_line" =~ (^|[^[:alnum:]])${sev}[[:space:]]*[:=][[:space:]]*([0-9]+) ]]; then
n="${BASH_REMATCH[2]}"
elif [[ "$clean_line" =~ (^|[^[:alnum:]])${sev}[[:space:]]*\(([0-9]+)\) ]]; then
n="${BASH_REMATCH[2]}"
elif [[ "$clean_line" =~ ([0-9]+)[[:space:]]+${sev}([^[:alnum:]]|$) ]]; then
n="${BASH_REMATCH[1]}"
fi
if [[ -n "$n" ]]; then last_n="$n"; fi
done <<<"$scope"
echo "$last_n"
}
# counts <file> → prints CRITICAL=n HIGH=n MEDIUM=n LOW=n ; exit 4 if C+H>0
# Parses the review's own "count by severity" block first (last such block in
# the file); only falls back to counting finding headings (marked "(derived)")
# when no count block yields any severity value at all.
counts() {
local f="$1"
[[ -f "$f" ]] || die "result file not found: $f" 2
local scope; scope=$(_counts_scope "$f")
local -A vals=()
local sev found_any=0
for sev in CRITICAL HIGH MEDIUM LOW; do
local n; n=$(_counts_extract "$scope" "$sev")
if [[ -n "$n" ]]; then vals[$sev]="$n"; found_any=1; fi
done
local derived=""
if [[ "$found_any" -eq 0 ]]; then
derived=" (derived)"
for sev in CRITICAL HIGH MEDIUM LOW; do
local n
n=$(grep -cE "^[[:space:]]*(#+[[:space:]]*)?(\*\*)?${sev}(\*\*)?[[:space:]]*(:|—|-)" "$f") || n=0
vals[$sev]="$n"
done
# CR-9: Suspect derived zero. If the derived fallback ran and yielded
# CRITICAL=0 and HIGH=0, but the file contains a CRITICAL/HIGH section
# header anywhere (case-insensitive), the derived count is untrustworthy
# — mark "(suspect)" instead of "(derived)" and force exit 4. Suspect
# and genuine C+H>0 share exit 4; distinguish via the marker.
if [[ "${vals[CRITICAL]:-0}" -eq 0 && "${vals[HIGH]:-0}" -eq 0 ]]; then
if grep -qiE "^#{1,6}[[:space:]]*(\*\*)?(CRITICAL|HIGH)\b" "$f"; then
derived=" (suspect)"
fi
fi
fi
local out="" total=0
for sev in CRITICAL HIGH MEDIUM LOW; do
local n="${vals[$sev]:-0}"
out+="${sev}=${n} "
if [[ "$sev" == "CRITICAL" || "$sev" == "HIGH" ]]; then total=$((total + n)); fi
done
echo "${out% }${derived}"
if [[ "$total" -gt 0 || "$derived" == " (suspect)" ]]; 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 "$@"