From aecc8fb8adb1f39f65903c9bf70589f006e52d7a Mon Sep 17 00:00:00 2001 From: openhands Date: Fri, 20 Mar 2026 22:59:02 +0000 Subject: [PATCH] fix: feat: migrate review-agent to formula architecture (#267) Co-Authored-By: Claude Opus 4.6 (1M context) --- formulas/review-pr.toml | 142 ++++++ review/review-pr.sh | 1009 ++++++--------------------------------- 2 files changed, 295 insertions(+), 856 deletions(-) create mode 100644 formulas/review-pr.toml diff --git a/formulas/review-pr.toml b/formulas/review-pr.toml new file mode 100644 index 0000000..636b361 --- /dev/null +++ b/formulas/review-pr.toml @@ -0,0 +1,142 @@ +# formulas/review-pr.toml — PR review formula +# +# Defines the review agent's judgment: understand the change, assess quality, +# decide verdict, write structured output. The bash orchestrator +# (review/review-pr.sh) handles session lifecycle, metadata fetching, +# API posting, and cleanup. +# +# The orchestrator injects PR context (diff, metadata, previous review) +# alongside this formula. Claude follows the steps in a single session. + +name = "review-pr" +description = "AI-powered PR review: understand change, assess quality, decide verdict" +version = 1 +model = "sonnet" + +[context] +files = ["AGENTS.md"] + +[[steps]] +id = "review" +title = "Review the PR and write structured output" +description = """ +You have full repo access — you are in a checkout of the PR branch. +Use this to verify claims, check existing code, and understand context +before flagging issues. Read files rather than guessing. + +## 1. Understand the change + +Read the diff and PR description injected by the orchestrator. +What is this PR doing and why? Identify the scope: +- Contracts, frontend, backend, docs, infra, formulas, mixed? +- New feature, bug fix, refactor, config change? + +## 2. CI relevance + +If CI has not passed, decide whether CI matters for this PR. +Non-code changes (docs, formulas, TOML config, markdown-only) do NOT +need CI — note this in your review rather than blocking. The orchestrator +already skips the CI gate for non-code PRs, but you should also mention +CI relevance in the review body when it applies. + +## 3. Review checklist + +Adapt based on scope. Check for: + +- **Bugs & logic errors**: off-by-one, nil/null dereference, missing edge cases, + wrong return type, incorrect conditionals +- **Security**: command injection, unquoted bash variables, path traversal, XSS, + secret leakage in logs or comments +- **Imports & dependencies**: broken imports, undefined variables, missing deps +- **Architecture**: verify patterns match AGENTS.md and project conventions +- **Bash specifics** (for .sh files): ShellCheck compliance, set -euo pipefail, + proper quoting, error handling with || true where appropriate, no echo of secrets +- **Dead code**: unused variables, unreachable branches, leftover debug prints +- **Claim verification**: if docs/README/AGENTS.md changed, verify claims + against actual code — read the files to confirm + +Do NOT flag: +- Style preferences with no correctness impact +- Missing tests unless the change is clearly untested and risky +- Things that look wrong but actually work — verify by reading the code first +- Files that were truncated from the diff (the orchestrator notes truncation) + +## 4. Re-review (if previous review is provided) + +If the orchestrator injected a previous review and incremental diff: +1. For each finding in the previous review, check if it was addressed +2. Report status: fixed / not_fixed / partial with explanation +3. Check for new issues introduced by the fix commits +4. Be fair — if feedback was addressed well, acknowledge it + +Focus on the incremental diff for finding-by-finding status, but use the +full diff to check overall correctness. + +## 5. Follow-up issues (pre-existing tech debt) + +If you discover pre-existing issues (NOT introduced by this PR), create +tech-debt issues via API so they are tracked separately: + + # Look up tech-debt label ID (create if missing): + TECH_DEBT_ID=$(curl -sf -H "Authorization: token $CODEBERG_TOKEN" \ + "$CODEBERG_API/labels" | jq -r '.[] | select(.name=="tech-debt") | .id') + + if [ -z "$TECH_DEBT_ID" ]; then + TECH_DEBT_ID=$(curl -sf -X POST \ + -H "Authorization: token $CODEBERG_TOKEN" \ + -H "Content-Type: application/json" \ + "$CODEBERG_API/labels" \ + -d '{"name":"tech-debt","color":"#6B7280","description":"Pre-existing tech debt flagged by AI review"}' | jq -r '.id') + fi + + # Check for duplicate before creating: + EXISTING=$(curl -sf -H "Authorization: token $CODEBERG_TOKEN" \ + "$CODEBERG_API/issues?state=open&labels=tech-debt&limit=50" | \ + jq --arg t "TITLE" '[.[] | select(.title == $t)] | length') + + # Create only if no duplicate: + curl -sf -X POST -H "Authorization: token $CODEBERG_TOKEN" \ + -H "Content-Type: application/json" "$CODEBERG_API/issues" \ + -d '{"title":"...","body":"Flagged by AI reviewer in PR #NNN.\n\n## Problem\n...\n\n---\n*Auto-created from AI review*","labels":[TECH_DEBT_ID]}' + +Only create follow-ups for clear, actionable tech debt. Do not create +issues for minor style nits or speculative improvements. + +## 6. Verdict + +Choose one: +- **APPROVE**: Change is correct, complete, and follows conventions +- **REQUEST_CHANGES**: Has real issues that must be fixed before merge +- **DISCUSS**: Unclear intent, design question, or needs human conversation + +Bias toward APPROVE for small, correct changes. Use REQUEST_CHANGES only +for actual problems (bugs, security issues, broken functionality, missing +required behavior). Use DISCUSS sparingly. + +## 7. Output + +Write a single JSON object to the file path from REVIEW_OUTPUT_FILE. +Use jq to ensure proper JSON escaping of the markdown content: + + jq -n \ + --arg verdict "APPROVE" \ + --arg verdict_reason "one-line explanation" \ + --arg review_markdown "### Section\\n- **severity** \\`location\\`: description" \ + '{verdict: $verdict, verdict_reason: $verdict_reason, review_markdown: $review_markdown}' \ + > "$REVIEW_OUTPUT_FILE" + +The review_markdown field must contain the complete, formatted review in +markdown. Use ### headings for sections. Use this format for findings: + - **severity** `file:line`: description + +For a re-review, structure the markdown as: + ### Previous Findings + - finding summary → FIXED / NOT FIXED / PARTIAL: explanation + ### New Issues (if any) + - **severity** `location`: description + +After writing the JSON file, signal completion: + echo "PHASE:done" > "$PHASE_FILE" + +Then STOP and wait. The orchestrator will post your review to Codeberg. +""" diff --git a/review/review-pr.sh b/review/review-pr.sh index 3b7b808..b5b90a7 100755 --- a/review/review-pr.sh +++ b/review/review-pr.sh @@ -1,901 +1,198 @@ #!/usr/bin/env bash -# review-pr.sh — AI-powered PR review using persistent Claude tmux session -# +# shellcheck disable=SC2015,SC2016 +# review-pr.sh — Thin orchestrator for AI PR review (formula: formulas/review-pr.toml) # Usage: ./review-pr.sh [--force] -# -# Session lifecycle: -# 1. Creates/reuses tmux session: review-{project}-{pr} -# 2. Injects PR diff + review guidelines into interactive claude -# 3. Claude reviews, writes structured JSON to output file -# 4. Script posts review to Codeberg -# 5. Session stays alive for re-reviews and human questions -# -# Re-review (new commits pushed): -# Same session → Claude remembers previous findings, verifies they're addressed -# -# Review output: /tmp/{project}-review-output-{pr}.json -# Phase file: /tmp/review-session-{project}-{pr}.phase -# Session: review-{project}-{pr} (tmux) -# Peek: cat /tmp/-review-status -# Log: tail -f /review/review.log - set -euo pipefail - -# Load shared environment source "$(dirname "$0")/../lib/env.sh" source "$(dirname "$0")/../lib/ci-helpers.sh" - -# Auto-pull factory code to pick up merged fixes before any logic runs +source "$(dirname "$0")/../lib/agent-session.sh" git -C "$FACTORY_ROOT" pull --ff-only origin main 2>/dev/null || true PR_NUMBER="${1:?Usage: review-pr.sh [--force]}" FORCE="${2:-}" -# shellcheck disable=SC2034 -REPO="${CODEBERG_REPO}" -# shellcheck disable=SC2034 -REPO_ROOT="${PROJECT_REPO_ROOT}" - -# Bot account for posting reviews (separate user required for branch protection approvals) -API_BASE="${CODEBERG_API}" +API="${CODEBERG_API}" +LOGFILE="${FACTORY_ROOT}/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}" LOCKFILE="/tmp/${PROJECT_NAME}-review.lock" STATUSFILE="/tmp/${PROJECT_NAME}-review-status" -LOGDIR="${FACTORY_ROOT}/review" -LOGFILE="$LOGDIR/review.log" -MIN_MEM_MB=1500 MAX_DIFF=25000 -MAX_ATTEMPTS=2 -TMPDIR=$(mktemp -d) - -# Tmux session + review output protocol -SESSION_NAME="review-${PROJECT_NAME}-${PR_NUMBER}" -PHASE_FILE="/tmp/review-session-${PROJECT_NAME}-${PR_NUMBER}.phase" -REVIEW_OUTPUT_FILE="/tmp/${PROJECT_NAME}-review-output-${PR_NUMBER}.json" -# Thread map: use standard MATRIX_THREAD_MAP (shared with all agents) -REVIEW_WAIT_INTERVAL=10 # seconds between phase checks -REVIEW_WAIT_TIMEOUT=600 # 10 min max for a single review cycle - -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 "$TMPDIR" - rm -f "$LOCKFILE" "$STATUSFILE" - # tmux session persists for re-reviews and human questions -} +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"; } trap cleanup EXIT -# Log rotation (100KB + 1 archive) if [ -f "$LOGFILE" ] && [ "$(stat -c%s "$LOGFILE" 2>/dev/null || echo 0)" -gt 102400 ]; then mv "$LOGFILE" "$LOGFILE.old" - log "Log rotated" fi - -# Memory guard -AVAIL_MB=$(awk '/MemAvailable/{printf "%d", $2/1024}' /proc/meminfo) -if [ "$AVAIL_MB" -lt "$MIN_MEM_MB" ]; then - log "SKIP: only ${AVAIL_MB}MB available (need ${MIN_MEM_MB}MB)" - exit 0 -fi - -# Concurrency lock +AVAIL=$(awk '/MemAvailable/{printf "%d", $2/1024}' /proc/meminfo) +[ "$AVAIL" -lt 1500 ] && { log "SKIP: ${AVAIL}MB available"; exit 0; } if [ -f "$LOCKFILE" ]; then - LOCK_PID=$(cat "$LOCKFILE" 2>/dev/null || echo "") - if [ -n "$LOCK_PID" ] && kill -0 "$LOCK_PID" 2>/dev/null; then - log "SKIP: another review running (PID ${LOCK_PID})" - exit 0 - fi - log "Removing stale lock (PID ${LOCK_PID:-?})" + 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" - -# --- Tmux session helpers --- -wait_for_claude_ready() { - local timeout="${1:-120}" - local elapsed=0 - while [ "$elapsed" -lt "$timeout" ]; do - # Check for Claude prompt: ❯ (UTF-8) or fallback to $ at line start - local pane_out - pane_out=$(tmux capture-pane -t "${SESSION_NAME}" -p 2>/dev/null || true) - if printf '%s' "$pane_out" | grep -qE '❯|^\$' 2>/dev/null; then - return 0 - fi - sleep 2 - elapsed=$((elapsed + 2)) - done - log "WARNING: claude not ready after ${timeout}s — proceeding anyway" - return 1 -} - -inject_into_session() { - local text="$1" - local tmpfile - wait_for_claude_ready 120 || true - tmpfile=$(mktemp /tmp/review-inject-XXXXXX) - printf '%s' "$text" > "$tmpfile" - # All tmux calls guarded with || true: the session is external and may die - # between the has-session check and here; a non-zero exit must not abort - # the script under set -euo pipefail. - tmux load-buffer -b "review-inject-${PR_NUMBER}" "$tmpfile" || true - tmux paste-buffer -t "${SESSION_NAME}" -b "review-inject-${PR_NUMBER}" || true - sleep 0.5 - tmux send-keys -t "${SESSION_NAME}" "" Enter || true - tmux delete-buffer -b "review-inject-${PR_NUMBER}" 2>/dev/null || true - rm -f "$tmpfile" -} - -wait_for_review_output() { - local timeout="$REVIEW_WAIT_TIMEOUT" - local elapsed=0 - while [ "$elapsed" -lt "$timeout" ]; do - # Check phase file before sleeping (avoids mandatory delay on fast reviews) - if ! tmux has-session -t "${SESSION_NAME}" 2>/dev/null; then - log "ERROR: session died during review" - return 1 - fi - local phase - phase=$(head -1 "$PHASE_FILE" 2>/dev/null | tr -d '[:space:]' || true) - if [ "$phase" = "PHASE:review_complete" ]; then - return 0 - fi - sleep "$REVIEW_WAIT_INTERVAL" - elapsed=$((elapsed + REVIEW_WAIT_INTERVAL)) - done - log "ERROR: review did not complete within ${timeout}s" - return 1 -} - -# --- Fetch PR metadata --- status "fetching metadata" -PR_JSON=$(curl -sf -H "Authorization: token ${CODEBERG_TOKEN}" \ - "${API_BASE}/pulls/${PR_NUMBER}") - -PR_TITLE=$(echo "$PR_JSON" | jq -r '.title') -PR_BODY=$(echo "$PR_JSON" | jq -r '.body // ""') -PR_HEAD=$(echo "$PR_JSON" | jq -r '.head.ref') -PR_BASE=$(echo "$PR_JSON" | jq -r '.base.ref') -PR_SHA=$(echo "$PR_JSON" | jq -r '.head.sha') -PR_STATE=$(echo "$PR_JSON" | jq -r '.state') - +PR_JSON=$(curl -sf -H "Authorization: token ${CODEBERG_TOKEN}" "${API}/pulls/${PR_NUMBER}") +PR_TITLE=$(printf '%s' "$PR_JSON" | jq -r '.title') +PR_BODY=$(printf '%s' "$PR_JSON" | jq -r '.body // ""') +PR_HEAD=$(printf '%s' "$PR_JSON" | jq -r '.head.ref') +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}" - cd "$REPO_ROOT" - # Kill review session for non-open PR - tmux kill-session -t "${SESSION_NAME}" 2>/dev/null || true - git worktree remove "/tmp/${PROJECT_NAME}-review-${PR_NUMBER}" --force 2>/dev/null || true - rm -rf "/tmp/${PROJECT_NAME}-review-${PR_NUMBER}" 2>/dev/null || true - rm -f "${PHASE_FILE}" "${REVIEW_OUTPUT_FILE}" - exit 0 + 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 fi - -status "checking CI" CI_STATE=$(curl -sf -H "Authorization: token ${CODEBERG_TOKEN}" \ - "${API_BASE}/commits/${PR_SHA}/status" | jq -r '.state // "unknown"') - -if ! ci_passed "$CI_STATE"; then - if ci_required_for_pr "$PR_NUMBER"; then - log "SKIP: CI=${CI_STATE}" - exit 0 - fi - log "CI=${CI_STATE} but PR has no code files — skipping CI gate" -fi - -# --- Check for existing reviews --- -status "checking existing reviews" + "${API}/commits/${PR_SHA}/status" | jq -r '.state // "unknown"') +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 ALL_COMMENTS=$(codeberg_api_all "/issues/${PR_NUMBER}/comments") - -# Check review-comment watermarks — skip if a comment with exists -COMMENT_REVIEWED=$(echo "$ALL_COMMENTS" | \ - jq -r --arg sha "$PR_SHA" \ - '[.[] | select(.body | contains(""))] | length') - -if [ "${COMMENT_REVIEWED:-0}" -gt "0" ] && [ "$FORCE" != "--force" ]; then - log "SKIP: review comment exists for ${PR_SHA:0:7}" - exit 0 -fi - -# Check formal Codeberg reviews — skip if a non-stale review exists for this SHA -EXISTING=$(codeberg_api_all "/pulls/${PR_NUMBER}/reviews" | \ - jq -r --arg sha "$PR_SHA" \ - '[.[] | select(.commit_id == $sha) | select(.state != "COMMENT")] | length') - -if [ "${EXISTING:-0}" -gt "0" ] && [ "$FORCE" != "--force" ]; then - log "SKIP: formal review exists for ${PR_SHA:0:7}" - exit 0 -fi - -# Find previous review for re-review mode -PREV_REVIEW_JSON=$(echo "$ALL_COMMENTS" | \ - jq -r --arg sha "$PR_SHA" \ - '[.[] | select(.body | contains(""))]|length') +[ "${HAS_CMT:-0}" -gt 0 ] && [ "$FORCE" != "--force" ] && { log "SKIP: reviewed ${PR_SHA:0:7}"; exit 0; } +HAS_FML=$(codeberg_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; } +PREV_CONTEXT="" IS_RE_REVIEW=false PREV_SHA="" +PREV_REV=$(printf '%s' "$ALL_COMMENTS" | jq -r --arg s "$PR_SHA" \ + '[.[]|select(.body|contains(" - -Review failed: could not produce structured output after ${MAX_ATTEMPTS} attempts. - -A maintainer should review this PR manually, or re-trigger with \`--force\`. - ---- -*Failed at \`${PR_SHA:0:7}\`*" - - printf '%s' "$ERROR_BODY" > "${TMPDIR}/comment-body.txt" - jq -Rs '{body: .}' < "${TMPDIR}/comment-body.txt" > "${TMPDIR}/comment.json" - - curl -s -o /dev/null -w "%{http_code}" \ - -X POST \ - -H "Authorization: token ${CODEBERG_TOKEN}" \ - -H "Content-Type: application/json" \ - "${API_BASE}/issues/${PR_NUMBER}/comments" \ - --data-binary @"${TMPDIR}/comment.json" > /dev/null - - # Save raw outputs for debugging - for f in "${LOGDIR}"/review-pr"${PR_NUMBER}"-raw-attempt-*.txt; do - [ -f "$f" ] && log "raw output saved: $f" - done - - matrix_send "review" "PR #${PR_NUMBER} review failed — no valid JSON output" 2>/dev/null || true - - exit 1 -fi - -# --- Render JSON -> Markdown --- -VERDICT=$(printf '%s' "$REVIEW_JSON" | jq -r '.verdict' | tr '[:lower:]' '[:upper:]' | tr '-' '_') -VERDICT_REASON=$(printf '%s' "$REVIEW_JSON" | jq -r '.verdict_reason // ""') - -render_markdown() { - local json="$1" - local md="" - - if [ "$IS_RE_REVIEW" = true ]; then - # Re-review format - local prev_count - prev_count=$(printf '%s' "$json" | jq '.previous_findings | length') - - if [ "$prev_count" -gt 0 ]; then - md+="### Previous Findings"$'\n' - while IFS= read -r finding; do - local summary finding_status explanation - summary=$(printf '%s' "$finding" | jq -r '.summary') - finding_status=$(printf '%s' "$finding" | jq -r '.status') - explanation=$(printf '%s' "$finding" | jq -r '.explanation') - - local icon="?" - case "$finding_status" in - fixed) icon="FIXED" ;; - not_fixed) icon="NOT FIXED" ;; - partial) icon="PARTIAL" ;; - esac - - md+="- ${summary} -> ${icon} ${explanation}"$'\n' - done < <(printf '%s' "$json" | jq -c '.previous_findings[]') - md+=$'\n' - fi - - local new_count - new_count=$(printf '%s' "$json" | jq '.new_issues | length') - if [ "$new_count" -gt 0 ]; then - md+="### New Issues"$'\n' - while IFS= read -r issue; do - local sev loc desc - sev=$(printf '%s' "$issue" | jq -r '.severity') - loc=$(printf '%s' "$issue" | jq -r '.location') - desc=$(printf '%s' "$issue" | jq -r '.description') - - md+="- **${sev}** \`${loc}\`: ${desc}"$'\n' - done < <(printf '%s' "$json" | jq -c '.new_issues[]') - md+=$'\n' - fi - - else - # Fresh review format - while IFS= read -r section; do - local title - title=$(printf '%s' "$section" | jq -r '.title') - local finding_count - finding_count=$(printf '%s' "$section" | jq '.findings | length') - - md+="### ${title}"$'\n' - - if [ "$finding_count" -eq 0 ]; then - md+="No issues found."$'\n'$'\n' - else - while IFS= read -r finding; do - local sev loc desc - sev=$(printf '%s' "$finding" | jq -r '.severity') - loc=$(printf '%s' "$finding" | jq -r '.location') - desc=$(printf '%s' "$finding" | jq -r '.description') - - md+="- **${sev}** \`${loc}\`: ${desc}"$'\n' - done < <(printf '%s' "$section" | jq -c '.findings[]') - md+=$'\n' - fi - done < <(printf '%s' "$json" | jq -c '.sections[]') - fi - - # Follow-ups - local followup_count - followup_count=$(printf '%s' "$json" | jq '.followups | length') - if [ "$followup_count" -gt 0 ]; then - md+="### Follow-up Issues"$'\n' - while IFS= read -r fu; do - local fu_title fu_details - fu_title=$(printf '%s' "$fu" | jq -r '.title') - fu_details=$(printf '%s' "$fu" | jq -r '.details') - md+="- **${fu_title}**: ${fu_details}"$'\n' - done < <(printf '%s' "$json" | jq -c '.followups[]') - md+=$'\n' - fi - - # Verdict - md+="### Verdict"$'\n' - md+="**${VERDICT}** — ${VERDICT_REASON}"$'\n' - - printf '%s' "$md" } +monitor_phase_loop "$PHASE_FILE" 600 "review_cb" "$SESSION" -REVIEW_MD=$(render_markdown "$REVIEW_JSON") - -# --- Post review to Codeberg --- -status "posting to Codeberg" - -REVIEW_TYPE="Review" -if [ "$IS_RE_REVIEW" = true ]; then - ROUND=$(($(echo "$ALL_COMMENTS" | jq '[.[] | select(.body | contains(" - -${REVIEW_MD} - ---- -*Reviewed at \`${PR_SHA:0:7}\`$(if [ "$IS_RE_REVIEW" = true ]; then echo " | Previous: \`${PREV_REVIEW_SHA:0:7}\`"; fi) | [AGENTS.md](AGENTS.md)*" - -printf '%s' "$COMMENT_BODY" > "${TMPDIR}/comment-body.txt" -jq -Rs '{body: .}' < "${TMPDIR}/comment-body.txt" > "${TMPDIR}/comment.json" - -POST_CODE=$(curl -s -o "${TMPDIR}/post-response.txt" -w "%{http_code}" \ - -X POST \ - -H "Authorization: token ${REVIEW_BOT_TOKEN}" \ - -H "Content-Type: application/json" \ - "${API_BASE}/issues/${PR_NUMBER}/comments" \ - --data-binary @"${TMPDIR}/comment.json") - -if [ "${POST_CODE}" = "201" ]; then - log "POSTED comment to Codeberg (as review_bot)" - - # Submit formal Codeberg review (required for branch protection approval) - REVIEW_EVENT="COMMENT" - case "$VERDICT" in - APPROVE) REVIEW_EVENT="APPROVED" ;; - REQUEST_CHANGES|DISCUSS) REVIEW_EVENT="REQUEST_CHANGES" ;; - esac - - # Dismiss prior REQUEST_CHANGES reviews before posting APPROVED - if [ "$REVIEW_EVENT" = "APPROVED" ]; then - REVIEW_BOT_RESP=$(curl -sf \ - -H "Authorization: token ${REVIEW_BOT_TOKEN}" \ - "${API_BASE%%/repos*}/user" 2>/dev/null || true) - REVIEW_BOT_LOGIN="" - if [ -n "$REVIEW_BOT_RESP" ]; then - REVIEW_BOT_LOGIN=$(printf '%s' "$REVIEW_BOT_RESP" | jq -r '.login // empty') - fi - if [ -n "$REVIEW_BOT_LOGIN" ]; then - ALL_PR_REVIEWS=$(codeberg_api_all "/pulls/${PR_NUMBER}/reviews" "$REVIEW_BOT_TOKEN" || echo "[]") - while IFS= read -r review_id; do - DISMISS_CODE=$(curl -s -o /dev/null -w "%{http_code}" \ - -X POST \ - -H "Authorization: token ${REVIEW_BOT_TOKEN}" \ - -H "Content-Type: application/json" \ - "${API_BASE}/pulls/${PR_NUMBER}/reviews/${review_id}/dismissals" \ - -d '{"message":"Superseded by approval"}' || echo "000") - log "dismissed prior REQUEST_CHANGES review ${review_id} (HTTP ${DISMISS_CODE})" - done < <(printf '%s' "$ALL_PR_REVIEWS" | \ - jq -r --arg login "$REVIEW_BOT_LOGIN" \ - '.[] | select(.state == "REQUEST_CHANGES") | select(.user.login == $login) | .id') - else - log "WARNING: could not determine review bot login — skipping dismiss step" - fi - fi - - FORMAL_BODY="AI ${REVIEW_TYPE}: **${VERDICT}** — ${VERDICT_REASON}" - jq -n --arg body "$FORMAL_BODY" --arg event "$REVIEW_EVENT" --arg sha "$PR_SHA" \ - '{body: $body, event: $event, commit_id: $sha}' > "${TMPDIR}/formal-review.json" - - REVIEW_CODE=$(curl -s -o "${TMPDIR}/review-response.txt" -w "%{http_code}" \ - -X POST \ - -H "Authorization: token ${REVIEW_BOT_TOKEN}" \ - -H "Content-Type: application/json" \ - "${API_BASE}/pulls/${PR_NUMBER}/reviews" \ - --data-binary @"${TMPDIR}/formal-review.json") - - if [ "${REVIEW_CODE}" = "200" ]; then - log "SUBMITTED formal ${REVIEW_EVENT} review" +REVIEW_JSON="" +if [ -f "$OUTPUT_FILE" ]; then + RAW=$(cat "$OUTPUT_FILE") + if printf '%s' "$RAW" | jq -e '.verdict' >/dev/null 2>&1; then REVIEW_JSON="$RAW" else - log "WARNING: formal review failed (HTTP ${REVIEW_CODE}): $(head -c 200 "${TMPDIR}/review-response.txt" 2>/dev/null)" - # Non-fatal — the comment is already posted + EXT=$(printf '%s' "$RAW" | sed -n '/^```json/,/^```$/p' | sed '1d;$d') + [ -z "$EXT" ] && EXT=$(printf '%s' "$RAW" | sed -n '/^{/,/^}/p') + [ -n "${EXT:-}" ] && printf '%s' "$EXT" | jq -e '.verdict' >/dev/null 2>&1 && REVIEW_JSON="$EXT" fi -else - log "ERROR: Codeberg HTTP ${POST_CODE}: $(head -c 200 "${TMPDIR}/post-response.txt" 2>/dev/null)" - echo "$REVIEW_MD" > "${LOGDIR}/review-pr${PR_NUMBER}-${PR_SHA:0:7}.md" - log "Review saved to ${LOGDIR}/review-pr${PR_NUMBER}-${PR_SHA:0:7}.md" - exit 1 fi - -# --- Auto-create follow-up issues from JSON --- -FOLLOWUP_COUNT=$(printf '%s' "$REVIEW_JSON" | jq '.followups | length') -if [ "$FOLLOWUP_COUNT" -gt 0 ]; then - log "processing ${FOLLOWUP_COUNT} follow-up issues" - - TECH_DEBT_ID=$(curl -sf -H "Authorization: token ${CODEBERG_TOKEN}" \ - "${API_BASE}/labels" | jq -r '.[] | select(.name=="tech-debt") | .id') - - if [ -z "$TECH_DEBT_ID" ]; then - TECH_DEBT_ID=$(curl -sf -X POST \ - -H "Authorization: token ${CODEBERG_TOKEN}" \ - -H "Content-Type: application/json" \ - "${API_BASE}/labels" \ - -d '{"name":"tech-debt","color":"#6B7280","description":"Pre-existing tech debt flagged by AI review"}' | jq -r '.id') - fi - - CREATED_COUNT=0 - while IFS= read -r fu; do - FU_TITLE=$(printf '%s' "$fu" | jq -r '.title') - FU_DETAILS=$(printf '%s' "$fu" | jq -r '.details') - - # Check for duplicate - EXISTING=$(codeberg_api_all "/issues?state=open&labels=tech-debt" | \ - jq -r --arg t "$FU_TITLE" '[.[] | select(.title == $t)] | length') - - if [ "${EXISTING:-0}" -gt 0 ]; then - log "skip duplicate follow-up: ${FU_TITLE}" - continue - fi - - ISSUE_BODY="Flagged by AI reviewer in PR #${PR_NUMBER}. - -## Problem - -${FU_DETAILS} - ---- -*Auto-created from AI review of PR #${PR_NUMBER}*" - - printf '%s' "$ISSUE_BODY" > "${TMPDIR}/followup-body.txt" - jq -n \ - --arg title "$FU_TITLE" \ - --rawfile body "${TMPDIR}/followup-body.txt" \ - --argjson labels "[$TECH_DEBT_ID]" \ - '{title: $title, body: $body, labels: $labels}' > "${TMPDIR}/followup-issue.json" - - CREATED=$(curl -sf -X POST \ - -H "Authorization: token ${CODEBERG_TOKEN}" \ - -H "Content-Type: application/json" \ - "${API_BASE}/issues" \ - --data-binary @"${TMPDIR}/followup-issue.json" | jq -r '.number // empty') - - if [ -n "$CREATED" ]; then - log "created follow-up issue #${CREATED}: ${FU_TITLE}" - CREATED_COUNT=$((CREATED_COUNT + 1)) - fi - done < <(printf '%s' "$REVIEW_JSON" | jq -c '.followups[]') - - log "created ${CREATED_COUNT} follow-up issues total" +if [ -z "$REVIEW_JSON" ]; then + log "ERROR: no valid review output" + jq -n --arg b "## AI Review — Error\n\nReview failed.\n---\n*${PR_SHA:0:7}*" \ + '{body: $b}' | curl -sf -o /dev/null -X POST -H "Authorization: token ${CODEBERG_TOKEN}" \ + -H "Content-Type: application/json" "${API}/issues/${PR_NUMBER}/comments" -d @- || true + matrix_send "review" "PR #${PR_NUMBER} review failed" 2>/dev/null || 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}" -# --- Notify Matrix (with thread mapping for human questions) --- -# Pass PR_NUMBER as context_tag (4th arg) so the standard thread map has it in column 4 -matrix_send "review" "PR #${PR_NUMBER} ${REVIEW_TYPE}: ${VERDICT} — ${PR_TITLE}" "" "$PR_NUMBER" >/dev/null 2>&1 || true +status "posting review" +RTYPE="Review" +if [ "$IS_RE_REVIEW" = true ]; then + RTYPE="Re-review (round $(($(printf '%s' "$ALL_COMMENTS" | \ + jq '[.[]|select(.body|contains("\n\n%s\n\n### Verdict\n**%s** — %s\n\n---\n*Reviewed at `%s`%s | [AGENTS.md](AGENTS.md)*' \ + "$RTYPE" "$PR_SHA" "$REVIEW_MD" "$VERDICT" "$REASON" "${PR_SHA:0:7}" "$PREV_REF") +printf '%s' "$COMMENT_BODY" > "${REVIEW_TMPDIR}/body.txt" +jq -Rs '{body: .}' < "${REVIEW_TMPDIR}/body.txt" > "${REVIEW_TMPDIR}/comment.json" +POST_RC=$(curl -s -o /dev/null -w "%{http_code}" -X POST \ + -H "Authorization: token ${REVIEW_BOT_TOKEN}" -H "Content-Type: application/json" \ + "${API}/issues/${PR_NUMBER}/comments" --data-binary @"${REVIEW_TMPDIR}/comment.json") +[ "$POST_RC" != "201" ] && { log "ERROR: comment HTTP ${POST_RC}"; exit 1; } +log "posted review comment" -log "DONE: ${VERDICT} (re-review: ${IS_RE_REVIEW})" +REVENT="COMMENT" +case "$VERDICT" in APPROVE) REVENT="APPROVED" ;; REQUEST_CHANGES|DISCUSS) REVENT="REQUEST_CHANGES" ;; esac +if [ "$REVENT" = "APPROVED" ]; then + BLOGIN=$(curl -sf -H "Authorization: token ${REVIEW_BOT_TOKEN}" \ + "${API%%/repos*}/user" 2>/dev/null | jq -r '.login // empty' || true) + [ -n "$BLOGIN" ] && codeberg_api_all "/pulls/${PR_NUMBER}/reviews" "$REVIEW_BOT_TOKEN" 2>/dev/null | \ + jq -r --arg l "$BLOGIN" '.[]|select(.state=="REQUEST_CHANGES")|select(.user.login==$l)|.id' | \ + while IFS= read -r rid; do + curl -sf -o /dev/null -X POST -H "Authorization: token ${REVIEW_BOT_TOKEN}" \ + -H "Content-Type: application/json" "${API}/pulls/${PR_NUMBER}/reviews/${rid}/dismissals" \ + -d '{"message":"Superseded by approval"}' || true; log "dismissed review ${rid}" + done || true +fi +jq -n --arg b "AI ${RTYPE}: **${VERDICT}** — ${REASON}" --arg e "$REVENT" --arg s "$PR_SHA" \ + '{body: $b, event: $e, commit_id: $s}' > "${REVIEW_TMPDIR}/formal.json" +curl -s -o /dev/null -X POST -H "Authorization: token ${REVIEW_BOT_TOKEN}" \ + -H "Content-Type: application/json" "${API}/pulls/${PR_NUMBER}/reviews" \ + --data-binary @"${REVIEW_TMPDIR}/formal.json" >/dev/null 2>&1 || true +log "formal ${REVENT} submitted" -# --- Write phase based on verdict --- -# Claude wrote PHASE:review_complete to signal JSON is ready; now overwrite -# with the correct lifecycle phase so review-poll.sh knows what to do next. +matrix_send "review" "PR #${PR_NUMBER} ${RTYPE}: ${VERDICT} — ${PR_TITLE}" "" "$PR_NUMBER" >/dev/null 2>&1 || true case "$VERDICT" in - APPROVE) - echo "PHASE:review_complete" > "${PHASE_FILE}" - # Terminal phase: kill session, clean up all associated files - tmux kill-session -t "${SESSION_NAME}" 2>/dev/null || true - rm -f "${PHASE_FILE}" "${REVIEW_OUTPUT_FILE}" \ - "/tmp/review-injected-${PROJECT_NAME}-${PR_NUMBER}" - cd "${REPO_ROOT}" - git worktree remove "${REVIEW_WORKTREE}" --force 2>/dev/null || true - rm -rf "${REVIEW_WORKTREE}" 2>/dev/null || true - ;; - REQUEST_CHANGES|DISCUSS) - printf 'PHASE:awaiting_changes\nSHA:%s\n' "$PR_SHA" > "${PHASE_FILE}" - log "awaiting new commits (PHASE:awaiting_changes)" - ;; - *) - echo "PHASE:review_complete" > "${PHASE_FILE}" - # Unknown verdict terminal phase: clean up like APPROVE - tmux kill-session -t "${SESSION_NAME}" 2>/dev/null || true - rm -f "${PHASE_FILE}" "${REVIEW_OUTPUT_FILE}" \ - "/tmp/review-injected-${PROJECT_NAME}-${PR_NUMBER}" - cd "${REPO_ROOT}" - git worktree remove "${REVIEW_WORKTREE}" --force 2>/dev/null || true - rm -rf "${REVIEW_WORKTREE}" 2>/dev/null || true - ;; + 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 ;; esac +log "DONE: ${VERDICT} (re-review: ${IS_RE_REVIEW})"