840d7509e1
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.
260 lines
12 KiB
Bash
Executable File
260 lines
12 KiB
Bash
Executable File
#!/usr/bin/env bash
|
|
# First-pass code review of a git branch in a MiniMax sub-session.
|
|
# The wrapper materialises the diff/log itself (the sub-session gets no Bash),
|
|
# runs MiniMax read-only except for one findings file, and gates the result.
|
|
# Spec: specs/review-minimax-run.spec.md. Used by /review-minimax; safe directly.
|
|
#
|
|
# Usage:
|
|
# review-minimax-run --worktree <path> [--base <ref>] [--out <findings-file>]
|
|
# [--profile agent-runtimes] [--max-diff-lines N] [--max-turns N]
|
|
# [--timeout SECS] [--dryrun|-n] [--m3] [--effort low|medium|high|xhigh|max]
|
|
#
|
|
# Exit codes:
|
|
# 0 findings file written (may contain "NO FINDINGS")
|
|
# 1 usage/config error, secrets root, credential-shaped or empty/oversize diff
|
|
# 2 MiniMax failed after retry, timeout, or sub-session dirtied the worktree
|
|
# 3 findings file missing or failed the result gate after retry
|
|
set -euo pipefail
|
|
|
|
usage() { grep '^#' "$0" | sed 's/^# \{0,1\}//' | sed -n '2,16p'; }
|
|
|
|
WORKTREE=""
|
|
BASE="main"
|
|
OUT_FILE=""
|
|
PROFILE=""
|
|
MAX_DIFF_LINES=6000
|
|
MAX_TURNS=40
|
|
TIMEOUT_SECS=540
|
|
USE_M3=0
|
|
EFFORT_ARG=""
|
|
DRYRUN=0
|
|
# RM-3: credential-shaped content in the diff aborts the review outright.
|
|
CRED_PATTERN='ghp_[A-Za-z0-9]|glpat-|x-access-token:|Authorization:|BEGIN [A-Z ]*PRIVATE KEY|AKIA[0-9A-Z]{16}'
|
|
|
|
while [[ $# -gt 0 ]]; do
|
|
case "$1" in
|
|
--worktree) WORKTREE="$2"; shift 2 ;;
|
|
--base) BASE="$2"; shift 2 ;;
|
|
--out) OUT_FILE="$2"; shift 2 ;;
|
|
--profile) PROFILE="$2"; shift 2 ;;
|
|
--max-diff-lines) MAX_DIFF_LINES="$2"; shift 2 ;;
|
|
--max-turns) MAX_TURNS="$2"; shift 2 ;;
|
|
--timeout) TIMEOUT_SECS="$2"; shift 2 ;;
|
|
--m3) USE_M3=1; shift ;;
|
|
--effort) EFFORT_ARG="$2"; shift 2 ;;
|
|
--dryrun|-n) DRYRUN=1; shift ;;
|
|
-h|--help) usage; exit 0 ;;
|
|
*) echo "error: unknown argument '$1'" >&2; usage >&2; exit 1 ;;
|
|
esac
|
|
done
|
|
if [[ $USE_M3 -eq 1 ]]; then MODEL="MiniMax-M3[1m]"; EFFORT="$EFFORT_ARG"; else MODEL="MiniMax-M3.1-Flash-Preview"; EFFORT="${EFFORT_ARG:-max}"; fi
|
|
[[ -z "$EFFORT" || "$EFFORT" =~ ^(low|medium|high|xhigh|max)$ ]] || { echo "error: --effort must be low|medium|high|xhigh|max" >&2; exit 1; }
|
|
|
|
# --- RM-1 preflight -----------------------------------------------------------
|
|
[[ -n "$WORKTREE" ]] || { echo "error: --worktree required" >&2; exit 1; }
|
|
command -v jq >/dev/null || { echo "error: jq not found" >&2; exit 1; }
|
|
command -v ask-minimax-run >/dev/null || { echo "error: ask-minimax-run not on PATH" >&2; exit 1; }
|
|
|
|
WORKTREE=$(realpath -e "$WORKTREE" 2>/dev/null) || { echo "error: worktree does not exist" >&2; exit 1; }
|
|
case "$WORKTREE" in
|
|
*/secrets|*/secrets/*|*/local_secrets|*/local_secrets/*)
|
|
echo "error: refusing secrets path '$WORKTREE'" >&2; exit 1 ;;
|
|
esac
|
|
git -C "$WORKTREE" rev-parse --show-toplevel >/dev/null 2>&1 || { echo "error: '$WORKTREE' is not a git checkout" >&2; exit 1; }
|
|
BRANCH=$(git -C "$WORKTREE" rev-parse --abbrev-ref HEAD)
|
|
[[ "$BRANCH" != "HEAD" ]] || { echo "error: worktree is on a detached HEAD" >&2; exit 1; }
|
|
git -C "$WORKTREE" rev-parse --verify --quiet "$BASE" >/dev/null || { echo "error: base ref '$BASE' does not resolve" >&2; exit 1; }
|
|
|
|
REPO_NAME=$(basename "$(git -C "$WORKTREE" rev-parse --show-toplevel)")
|
|
BRANCH_SLUG=${BRANCH//\//-}
|
|
if [[ -z "$OUT_FILE" ]]; then
|
|
OUT_FILE="${XDG_RUNTIME_DIR:-/tmp}/review-minimax/$REPO_NAME/$BRANCH_SLUG-findings.md"
|
|
fi
|
|
OUT_DIR=$(dirname "$OUT_FILE")
|
|
mkdir -p "$OUT_DIR"
|
|
|
|
WORK_DIR=$(mktemp -d /tmp/review-minimax.XXXXXX)
|
|
trap 'rm -rf "$WORK_DIR"' EXIT
|
|
|
|
# --- RM-2 materialise inputs (wrapper runs git, never the sub-session) --------
|
|
git -C "$WORKTREE" diff "$BASE...HEAD" -- \
|
|
':(exclude)secrets/**' ':(exclude)local_secrets/**' \
|
|
':(exclude)*.pem' ':(exclude)*.key' ':(exclude)*secret*' \
|
|
> "$WORK_DIR/diff.patch"
|
|
git -C "$WORKTREE" log --oneline "$BASE..HEAD" > "$WORK_DIR/commits.txt"
|
|
git -C "$WORKTREE" diff --stat "$BASE...HEAD" > "$WORK_DIR/stat.txt"
|
|
|
|
# --- RM-3 / RM-4 gates --------------------------------------------------------
|
|
if grep -qE "$CRED_PATTERN" "$WORK_DIR/diff.patch"; then
|
|
echo "error: diff contains credential-shaped content; review in-session" >&2
|
|
exit 1
|
|
fi
|
|
DIFF_LINES=$(wc -l < "$WORK_DIR/diff.patch")
|
|
[[ "$DIFF_LINES" -gt 0 ]] || { echo "error: empty diff $BASE...HEAD — nothing to review" >&2; exit 1; }
|
|
if [[ "$DIFF_LINES" -gt "$MAX_DIFF_LINES" ]]; then
|
|
echo "error: diff is $DIFF_LINES lines (> $MAX_DIFF_LINES); raise --max-diff-lines or review in-session" >&2
|
|
exit 1
|
|
fi
|
|
|
|
# --- RM-5 prompt --------------------------------------------------------------
|
|
FULL_PROMPT="$WORK_DIR/prompt.txt"
|
|
{
|
|
cat <<'PREAMBLE'
|
|
You are a FIRST-PASS CODE REVIEWER running non-interactively. Review the branch
|
|
diff provided as files in your working directory. Rules:
|
|
|
|
- Inputs (in cwd): diff.patch (the review scope), commits.txt, stat.txt.
|
|
- The worktree listed below is readable for SURROUNDING CONTEXT ONLY.
|
|
- Write ONLY the findings file at the path given below. Never modify, create,
|
|
or delete any other file anywhere.
|
|
- Do not run anything. Do not fix anything. Review only.
|
|
- If an input file is missing or truncated, write a findings file saying
|
|
exactly that and stop — do not improvise or fetch more.
|
|
- Every finding MUST cite file:line from the diff or worktree, quote at most
|
|
10 lines, and carry a severity and a confidence. Do NOT invent names, paths,
|
|
or behaviour — a claim you cannot cite is omitted, not guessed.
|
|
|
|
Review checklist — look specifically for these patterns:
|
|
- Injection: string-built SQL; subprocess with shell=True; unquoted shell
|
|
variables; eval/exec on external input; paths used without realpath checks.
|
|
- Credentials: hardcoded password/api_key/token assignments; secrets passed as
|
|
command-line arguments; verify=False on HTTP calls.
|
|
- Weak crypto: random.* for security values; md5/sha1 for passwords.
|
|
- Over-permissive: chmod 777/666; 0.0.0.0 binds without justification;
|
|
debug=True; privileged/cluster-admin in manifests; :latest or unpinned images.
|
|
- Supply chain: curl|bash or wget|sh without checksum; unpinned dependencies.
|
|
- Operational: webhook/POST handlers without idempotency or duplicate checks;
|
|
health endpoints returning 200 unconditionally; read-then-write without
|
|
transaction or lock; CI steps consuming prior artifacts without needs:.
|
|
- Error handling: exceptions swallowed silently; internal detail leaked to
|
|
clients; bare except around control flow.
|
|
- Tests in the diff: assertions that cannot fail; fixtures that bypass the
|
|
auth/dependency they claim to exercise; asserts on wrong response shape.
|
|
|
|
Findings file format (exactly this shape):
|
|
- One block per finding:
|
|
### [BLOCKER|SHOULD-FIX|NIT] <short title>
|
|
- Location: <file>:<line>
|
|
- Category: <checklist category>
|
|
- Confidence: confirmed|suspected
|
|
- Excerpt (<=10 lines) and one-sentence rationale.
|
|
- Order blocks most-severe first.
|
|
- If the diff is clean against the checklist, write exactly: NO FINDINGS
|
|
PREAMBLE
|
|
if [[ "$PROFILE" == "agent-runtimes" ]]; then
|
|
cat <<'PROFILEBLOCK'
|
|
|
|
Additional profile checklist (agent-runtimes test files):
|
|
- API test paths missing the /v1/ prefix (404s at runtime).
|
|
- Assertions on body["type"] instead of body["detail"]["type"] (RFC 9457 is
|
|
wrapped inside FastAPI's detail).
|
|
- Manifest bodies with {params, nodes, definition_version} at top level
|
|
instead of {project_id, space_id, definition_version, definition:{...}}.
|
|
- Subscript access on dataclass fixtures (obj["field"] instead of obj.field).
|
|
- Local auth-header fixture dicts that do not override the authenticate
|
|
dependency (cross-tenant checks silently pass as anonymous).
|
|
PROFILEBLOCK
|
|
fi
|
|
echo
|
|
echo "## Worktree (context reads only)"
|
|
echo "$WORKTREE (branch: $BRANCH, base: $BASE)"
|
|
echo
|
|
echo "## Findings file to write"
|
|
echo "$OUT_FILE"
|
|
} > "$FULL_PROMPT"
|
|
|
|
# --- dryrun -------------------------------------------------------------------
|
|
if [[ "$DRYRUN" -eq 1 ]]; then
|
|
echo "== review-minimax-run --dryrun"
|
|
echo "worktree: $WORKTREE"
|
|
echo "branch: $BRANCH"
|
|
echo "base: $BASE"
|
|
echo "out: $OUT_FILE"
|
|
echo "profile: ${PROFILE:-none}"
|
|
echo "diff lines: $DIFF_LINES (budget $MAX_DIFF_LINES)"
|
|
echo "max turns: $MAX_TURNS timeout: ${TIMEOUT_SECS}s"
|
|
echo "-- stat --"
|
|
cat "$WORK_DIR/stat.txt"
|
|
echo "-- prompt --"
|
|
cat "$FULL_PROMPT"
|
|
echo "-- command --"
|
|
echo "cd $WORK_DIR && timeout $TIMEOUT_SECS ask-minimax-run --model '$MODEL'${EFFORT:+ --effort $EFFORT}" \
|
|
"--permission-mode default --allowedTools 'Read,Glob,Grep,Write'" \
|
|
"--disallowedTools 'Edit,MultiEdit,NotebookEdit,Bash,WebFetch,WebSearch'" \
|
|
"--max-turns $MAX_TURNS --output-format json --add-dir $WORKTREE --add-dir $OUT_DIR -p < prompt.txt"
|
|
exit 0
|
|
fi
|
|
|
|
# --- RM-6 invocation ----------------------------------------------------------
|
|
run_once() {
|
|
local json_out="$1"
|
|
if [[ -n "${REVIEW_MINIMAX_FAKE_JSON:-}" ]]; then # RM-11 test seam
|
|
cp "$REVIEW_MINIMAX_FAKE_JSON" "$json_out"
|
|
# optional companion seam: plays the sub-session's Write of the findings file
|
|
[[ -n "${REVIEW_MINIMAX_FAKE_FINDINGS:-}" ]] && cp "$REVIEW_MINIMAX_FAKE_FINDINGS" "$OUT_FILE"
|
|
return 0
|
|
fi
|
|
( cd "$WORK_DIR" && timeout "$TIMEOUT_SECS" ask-minimax-run \
|
|
--model "$MODEL" ${EFFORT:+--effort "$EFFORT"} \
|
|
--permission-mode default \
|
|
--allowedTools "Read,Glob,Grep,Write" \
|
|
--disallowedTools "Edit,MultiEdit,NotebookEdit,Bash,WebFetch,WebSearch" \
|
|
--max-turns "$MAX_TURNS" \
|
|
--output-format json \
|
|
--add-dir "$WORKTREE" \
|
|
--add-dir "$OUT_DIR" \
|
|
-p < "$FULL_PROMPT" > "$json_out" 2>"$WORK_DIR/stderr.txt" )
|
|
}
|
|
|
|
tree_clean() { [[ -z "$(git -C "$WORKTREE" status --porcelain)" ]]; }
|
|
|
|
gate_ok() { # RM-8: strict — severity block + resolvable citation, or NO FINDINGS
|
|
[[ -f "$OUT_FILE" ]] || return 1
|
|
grep -q 'NO FINDINGS' "$OUT_FILE" && return 0
|
|
grep -qE '^### \[(BLOCKER|SHOULD-FIX|NIT)\]' "$OUT_FILE" || return 1
|
|
local cited
|
|
while read -r cited; do
|
|
[[ -e "$WORKTREE/$cited" || -e "$cited" ]] && return 0
|
|
done < <(grep -oE 'Location: [^:[:space:]]+' "$OUT_FILE" | sed 's/^Location: //')
|
|
return 1
|
|
}
|
|
|
|
JSON_OUT="$WORK_DIR/response.json"
|
|
RUN_FAILED=0
|
|
for attempt in 1 2; do
|
|
rm -f "$OUT_FILE" # RM-9: a stale attempt-1 file must not satisfy the gate
|
|
RUN_FAILED=0
|
|
rc=0
|
|
run_once "$JSON_OUT" || rc=$?
|
|
if [[ "$rc" -eq 124 ]]; then
|
|
echo "error: sub-session timed out after ${TIMEOUT_SECS}s (not retried)" >&2
|
|
exit 2
|
|
fi
|
|
[[ "$rc" -ne 0 ]] && RUN_FAILED=1
|
|
jq -e 'select(.is_error == true)' "$JSON_OUT" >/dev/null 2>&1 && RUN_FAILED=1
|
|
if ! tree_clean; then # RM-7
|
|
echo "error: sub-session dirtied the worktree — inspect: git -C $WORKTREE status" >&2
|
|
exit 2
|
|
fi
|
|
[[ "$RUN_FAILED" -eq 0 ]] && gate_ok && break
|
|
if [[ $attempt -eq 1 ]]; then
|
|
echo "warn: attempt 1 failed ($(tail -1 "$WORK_DIR/stderr.txt" 2>/dev/null || echo 'no stderr')); retrying once" >&2
|
|
fi
|
|
done
|
|
|
|
if [[ "$RUN_FAILED" -eq 1 ]]; then
|
|
echo "error: MiniMax failed after retry" >&2
|
|
tail -5 "$WORK_DIR/stderr.txt" >&2 2>/dev/null || true
|
|
exit 2
|
|
fi
|
|
if ! gate_ok; then
|
|
echo "error: findings file missing or failed the result gate after retry" >&2
|
|
exit 3
|
|
fi
|
|
|
|
# --- RM-10 telemetry ----------------------------------------------------------
|
|
jq -r '"telemetry: window=\(.modelUsage // {} | to_entries[0].value.contextWindow // "?") in=\(.usage.input_tokens // "?") out=\(.usage.output_tokens // "?") ms=\(.duration_ms // "?")"' \
|
|
"$JSON_OUT" >&2 2>/dev/null || true
|
|
|
|
echo "ok: findings written to $OUT_FILE"
|