diff --git a/review/review-pr.sh b/review/review-pr.sh index fd41404..00cf689 100755 --- a/review/review-pr.sh +++ b/review/review-pr.sh @@ -1,41 +1,115 @@ #!/usr/bin/env bash # shellcheck disable=SC2015,SC2016 -# review-pr.sh — Thin orchestrator for AI PR review (formula: formulas/review-pr.toml) +# review-pr.sh — Synchronous reviewer agent for a single PR +# # Usage: ./review-pr.sh [--force] +# +# Architecture: +# Synchronous bash loop using claude -p (one-shot invocations). +# Session continuity via --resume and .sid file. +# Re-review resumes the original session — Claude remembers its prior review. +# +# Flow: +# 1. Fetch PR metadata (title, body, head, base, SHA, CI state) +# 2. Detect re-review (previous review at different SHA, incremental diff) +# 3. Create review worktree, checkout PR head +# 4. Build structural analysis graph +# 5. Load review formula +# 6. agent_run(worktree, prompt) → Claude reviews, writes verdict JSON +# 7. Parse verdict, post as Forge review (APPROVE / REQUEST_CHANGES / COMMENT) +# 8. Save session ID to .sid file for re-review continuity +# +# Session file: /tmp/review-session-{project}-{pr}.sid set -euo pipefail + +# Load shared environment and libraries source "$(dirname "$0")/../lib/env.sh" source "$(dirname "$0")/../lib/ci-helpers.sh" -source "$(dirname "$0")/../lib/agent-session.sh" +source "$(dirname "$0")/../lib/worktree.sh" + +# Auto-pull factory code to pick up merged fixes before any logic runs git -C "$FACTORY_ROOT" pull --ff-only origin main 2>/dev/null || true +# --- Config --- PR_NUMBER="${1:?Usage: review-pr.sh [--force]}" FORCE="${2:-}" API="${FORGE_API}" LOGFILE="${DISINTO_LOG_DIR}/review/review.log" -SESSION="review-${PROJECT_NAME}-${PR_NUMBER}" -PHASE_FILE="/tmp/review-session-${PROJECT_NAME}-${PR_NUMBER}.phase" -OUTPUT_FILE="/tmp/${PROJECT_NAME}-review-output-${PR_NUMBER}.json" WORKTREE="/tmp/${PROJECT_NAME}-review-${PR_NUMBER}" +SID_FILE="/tmp/review-session-${PROJECT_NAME}-${PR_NUMBER}.sid" +OUTPUT_FILE="/tmp/${PROJECT_NAME}-review-output-${PR_NUMBER}.json" LOCKFILE="/tmp/${PROJECT_NAME}-review.lock" STATUSFILE="/tmp/${PROJECT_NAME}-review-status" MAX_DIFF=25000 REVIEW_TMPDIR=$(mktemp -d) + log() { printf '[%s] PR#%s %s\n' "$(date -u '+%Y-%m-%d %H:%M:%S UTC')" "$PR_NUMBER" "$*" >> "$LOGFILE"; } status() { printf '[%s] PR #%s: %s\n' "$(date -u '+%Y-%m-%d %H:%M:%S UTC')" "$PR_NUMBER" "$*" > "$STATUSFILE"; log "$*"; } cleanup() { rm -rf "$REVIEW_TMPDIR" "$LOCKFILE" "$STATUSFILE" "/tmp/${PROJECT_NAME}-review-graph-${PR_NUMBER}.json"; } trap cleanup EXIT +# ============================================================================= +# agent_run — synchronous Claude invocation (one-shot claude -p) +# ============================================================================= +# Usage: agent_run [--resume SESSION_ID] [--worktree DIR] PROMPT +# Sets: _AGENT_SESSION_ID (updated each call, persisted to SID_FILE) +_AGENT_SESSION_ID="" + +agent_run() { + local resume_id="" worktree_dir="" + while [[ "${1:-}" == --* ]]; do + case "$1" in + --resume) shift; resume_id="${1:-}"; shift ;; + --worktree) shift; worktree_dir="${1:-}"; shift ;; + *) shift ;; + esac + done + local prompt="${1:-}" + + local -a args=(-p "$prompt" --output-format json --dangerously-skip-permissions --max-turns 200) + [ -n "$resume_id" ] && args+=(--resume "$resume_id") + [ -n "${CLAUDE_MODEL:-}" ] && args+=(--model "$CLAUDE_MODEL") + + local run_dir="${worktree_dir:-$(pwd)}" + local output + log "agent_run: starting (resume=${resume_id:-(new)}, dir=${run_dir})" + output=$(cd "$run_dir" && timeout "${CLAUDE_TIMEOUT:-7200}" claude "${args[@]}" 2>>"$LOGFILE") || true + + # Extract and persist session_id + local new_sid + new_sid=$(printf '%s' "$output" | jq -r '.session_id // empty' 2>/dev/null) || true + if [ -n "$new_sid" ]; then + _AGENT_SESSION_ID="$new_sid" + printf '%s' "$new_sid" > "$SID_FILE" + log "agent_run: session_id=${new_sid:0:12}..." + fi +} + +# ============================================================================= +# LOG ROTATION +# ============================================================================= if [ -f "$LOGFILE" ] && [ "$(stat -c%s "$LOGFILE" 2>/dev/null || echo 0)" -gt 102400 ]; then mv "$LOGFILE" "$LOGFILE.old" fi -AVAIL=$(awk '/MemAvailable/{printf "%d", $2/1024}' /proc/meminfo) -[ "$AVAIL" -lt 1500 ] && { log "SKIP: ${AVAIL}MB available"; exit 0; } + +# ============================================================================= +# MEMORY GUARD +# ============================================================================= +memory_guard 1500 + +# ============================================================================= +# CONCURRENCY LOCK +# ============================================================================= if [ -f "$LOCKFILE" ]; then LPID=$(cat "$LOCKFILE" 2>/dev/null || true) [ -n "$LPID" ] && kill -0 "$LPID" 2>/dev/null && { log "SKIP: locked"; exit 0; } rm -f "$LOCKFILE" fi echo $$ > "$LOCKFILE" + +# ============================================================================= +# FETCH PR METADATA +# ============================================================================= status "fetching metadata" PR_JSON=$(curl -sf -H "Authorization: token ${FORGE_TOKEN}" "${API}/pulls/${PR_NUMBER}") PR_TITLE=$(printf '%s' "$PR_JSON" | jq -r '.title') @@ -45,15 +119,27 @@ PR_BASE=$(printf '%s' "$PR_JSON" | jq -r '.base.ref') PR_SHA=$(printf '%s' "$PR_JSON" | jq -r '.head.sha') PR_STATE=$(printf '%s' "$PR_JSON" | jq -r '.state') log "${PR_TITLE} (${PR_HEAD}→${PR_BASE} ${PR_SHA:0:7})" + if [ "$PR_STATE" != "open" ]; then - log "SKIP: state=${PR_STATE}"; agent_kill_session "$SESSION" - cd "${PROJECT_REPO_ROOT}"; git worktree remove "$WORKTREE" --force 2>/dev/null || true - rm -rf "$WORKTREE" "$PHASE_FILE" "$OUTPUT_FILE" 2>/dev/null || true; exit 0 + log "SKIP: state=${PR_STATE}" + worktree_cleanup "$WORKTREE" + rm -f "$OUTPUT_FILE" "$SID_FILE" 2>/dev/null || true + exit 0 fi + +# ============================================================================= +# CI CHECK +# ============================================================================= CI_STATE=$(ci_commit_status "$PR_SHA") -CI_NOTE=""; if ! ci_passed "$CI_STATE"; then +CI_NOTE="" +if ! ci_passed "$CI_STATE"; then ci_required_for_pr "$PR_NUMBER" && { log "SKIP: CI=${CI_STATE}"; exit 0; } - CI_NOTE=" (not required — non-code PR)"; fi + CI_NOTE=" (not required — non-code PR)" +fi + +# ============================================================================= +# DUPLICATE CHECK — skip if already reviewed at this SHA +# ============================================================================= ALL_COMMENTS=$(forge_api_all "/issues/${PR_NUMBER}/comments") HAS_CMT=$(printf '%s' "$ALL_COMMENTS" | jq --arg s "$PR_SHA" \ '[.[]|select(.body|contains(""))]|length') @@ -61,6 +147,10 @@ HAS_CMT=$(printf '%s' "$ALL_COMMENTS" | jq --arg s "$PR_SHA" \ HAS_FML=$(forge_api_all "/pulls/${PR_NUMBER}/reviews" | jq --arg s "$PR_SHA" \ '[.[]|select(.commit_id==$s)|select(.state!="COMMENT")]|length') [ "${HAS_FML:-0}" -gt 0 ] && [ "$FORCE" != "--force" ] && { log "SKIP: formal review"; exit 0; } + +# ============================================================================= +# RE-REVIEW DETECTION +# ============================================================================= PREV_CONTEXT="" IS_RE_REVIEW=false PREV_SHA="" PREV_REV=$(printf '%s' "$ALL_COMMENTS" | jq -r --arg s "$PR_SHA" \ '[.[]|select(.body|contains("\nReview failed.\n---\n*${PR_SHA:0:7}*" \ @@ -162,11 +274,15 @@ if [ -z "$REVIEW_JSON" ]; then -H "Content-Type: application/json" "${API}/issues/${PR_NUMBER}/comments" -d @- || true exit 1 fi + VERDICT=$(printf '%s' "$REVIEW_JSON" | jq -r '.verdict' | tr '[:lower:]' '[:upper:]' | tr '-' '_') REASON=$(printf '%s' "$REVIEW_JSON" | jq -r '.verdict_reason // ""') REVIEW_MD=$(printf '%s' "$REVIEW_JSON" | jq -r '.review_markdown // ""') log "verdict: ${VERDICT}" +# ============================================================================= +# POST REVIEW +# ============================================================================= status "posting review" RTYPE="Review" if [ "$IS_RE_REVIEW" = true ]; then @@ -184,6 +300,9 @@ POST_RC=$(curl -s -o /dev/null -w "%{http_code}" -X POST \ [ "$POST_RC" != "201" ] && { log "ERROR: comment HTTP ${POST_RC}"; exit 1; } log "posted review comment" +# ============================================================================= +# POST FORMAL REVIEW +# ============================================================================= REVENT="COMMENT" case "$VERDICT" in APPROVE) REVENT="APPROVED" ;; REQUEST_CHANGES|DISCUSS) REVENT="REQUEST_CHANGES" ;; esac if [ "$REVENT" = "APPROVED" ]; then @@ -204,10 +323,18 @@ curl -s -o /dev/null -X POST -H "Authorization: token ${FORGE_REVIEW_TOKEN}" \ --data-binary @"${REVIEW_TMPDIR}/formal.json" >/dev/null 2>&1 || true log "formal ${REVENT} submitted" +# ============================================================================= +# FINAL CLEANUP +# ============================================================================= case "$VERDICT" in - REQUEST_CHANGES|DISCUSS) printf 'PHASE:awaiting_changes\nSHA:%s\n' "$PR_SHA" > "$PHASE_FILE" ;; - *) rm -f "$PHASE_FILE" "$OUTPUT_FILE"; cd "${PROJECT_REPO_ROOT}" - git worktree remove "$WORKTREE" --force 2>/dev/null || true - rm -rf "$WORKTREE" 2>/dev/null || true ;; + REQUEST_CHANGES|DISCUSS) + # Keep session and worktree for re-review continuity + log "keeping session for re-review (SID: ${_AGENT_SESSION_ID:0:12}...)" + ;; + *) + rm -f "$SID_FILE" "$OUTPUT_FILE" + worktree_cleanup "$WORKTREE" + ;; esac + log "DONE: ${VERDICT} (re-review: ${IS_RE_REVIEW})"