Post-review.sh should retry on transient GitHub API errors when submitting formal PR reviews

[fullsend-ai-retro]

What happened

On PR #148, the merge queue ejected the PR because the 001-clean-approve review eval case failed. The review agent performed correctly — it approved, posted a thorough comment (scored 5/5 on review_quality), and stayed within cost/turn limits. But post-review.sh received a 422 Unprocessable Entity from the GitHub API when submitting the formal PR review to the ephemeral fixture repo (halfsend/eval-001-clean-approve-1400fa2d). This caused post-review.sh to exit 1, which meant the ready-for-merge label was never applied. The required_labels judge then failed because ready-for-merge was absent. The human had to diagnose the root cause from CI logs, post a detailed analysis, and re-queue the PR, adding ~45 minutes to merge time.

What could go better

The human noted this is not just an eval concern: 'If post-review.sh fails hard on a 422, the same thing will happen in production: a valid review gets posted as a comment but the label never lands, so the PR sits unmerged. That needs separate follow-up (error handling / retry in post-review.sh).' The 422 can occur when the PR is in a transitional state (e.g., just created, merging, or when the head SHA changes during the review submission window). No existing issue covers this failure mode. Confidence is high — the human's analysis is credible and the fix is straightforward.

Proposed change

Add retry logic with backoff to the formal PR review submission step in scripts/post-review.sh. Specifically: (1) wrap the gh api or gh pr review call that submits the formal review in a retry loop (max 3 attempts with 5-second/15-second backoff), (2) on 422, log the response body for debugging and retry, (3) if all retries are exhausted but the review comment was already posted successfully, attempt to apply the ready-for-merge label directly as a degraded-mode fallback, (4) include the HTTP status code and response body in the error message for diagnosability. This improves both eval reliability (no false eval failures from transient API errors) and production reliability (reviews don't silently fail to trigger merge labels).

Validation criteria

When post-review.sh encounters a 422 on formal PR review submission: (1) it retries up to 3 times with backoff, (2) retry attempts are logged with the response body, (3) if retries succeed, the label is applied normally, (4) if retries are exhausted but the comment was posted, it attempts to apply the label directly. Unit test: add a test case to scripts/post-review-test.sh that mocks a 422 response and verifies the retry behavior.

---

Generated by retro agent from #148

[fullsend-ai-triage]

🤖 Finished Triage · ✅ Success · Started 7:40 PM UTC · Completed 7:45 PM UTC
Commit: bd28f41 · View workflow run →

[fullsend-ai-triage]

Triage Summary

Severity: Medium · Category: Bug

Problem

When fullsend post-review encounters a transient GitHub API error (specifically 422 Unprocessable Entity), post-review.sh exits immediately without retrying. The review content may already be posted as a comment, but the outcome label (ready-for-merge) is never applied because the script exits before reaching the label-application section. This was observed on PR #148, where the merge queue ejected the PR and required ~45 minutes of human diagnosis.

Root Cause Hypothesis

The fullsend post-review call has no retry logic. The script handles exit code 10 (stale-head) with a re-dispatch mechanism, but all other non-zero exit codes cause a hard failure. GitHub returns 422 when a PR is in a transitional state (just created, head SHA changed during submission window, etc.).

Recommended Fix

1. Wrap the fullsend post-review call in a retry loop (max 3 attempts, 5s/15s backoff)
2. On non-zero, non-10 exit codes: log the failure and retry
3. If all retries are exhausted but the review comment was already posted: apply the outcome label directly as a degraded-mode fallback
4. Log response details at each retry for diagnosability
5. Exit code 10 (stale-head) should bypass the retry loop — it already has dedicated handling

Note: the script uses fullsend post-review (not gh api or gh pr review directly), so retry logic wraps that CLI call.

Proposed Test Case

Extend scripts/post-review-test.sh with:

1. Retry-then-succeed: mock fullsend post-review to fail twice
   then succeed → verify correct label is applied
2. Retries exhausted + fallback: mock all 3 attempts failing
   → verify fallback label application is attempted
3. Stale-head bypass: mock exit code 10 → verify no retry loop,
   existing stale-head handler runs
4. Logging: verify each retry logs attempt number and error context

Related Issues

• Post-script push error handler should detect workflow-permission rejections before retrying #139 (push error handler retry detection) — similar retry pattern in post-fix.sh
• Extract CSMA library to scripts/lib/ to eliminate duplication #70 (CSMA library extraction) — shared retry/resilience library patterns

[fullsend-ai-coder]

🤖 Finished Code · ✅ Success · Started 7:45 PM UTC · Completed 8:04 PM UTC
Commit: bd28f41 · View workflow run →

[rh-hemartin]

This fix belongs to fullsend CLI.