fix(#159): retry fullsend post-review on transient API failures

[fullsend-ai-coder]

Wrap the fullsend post-review call in a retry loop (max 3 attempts with 5s/15s backoff) to handle transient GitHub API errors such as 422 Unprocessable Entity during PR transitional states.

Previously, a single non-zero exit (other than code 10 for stale-head) caused post-review.sh to exit immediately. This meant a valid review comment could be posted while the formal review API call failed, leaving the PR without an outcome label and requiring human diagnosis.

Changes:

• scripts/post-review.sh: add retry loop around fullsend post-review.
  Exit code 10 (stale-head) bypasses retry and uses existing handler.
  On exhausted retries, a degraded-mode fallback applies the outcome
  label directly so the PR is not left in limbo. The script still
  exits 1 to signal the formal review was not posted.
• scripts/post-review-test.sh: add 7 integration tests covering
  retry-then-succeed, retries-exhausted with fallback for approve/
  comment/request-changes actions, stale-head bypass, and retry
  log message format.

POST_REVIEW_RETRY_DELAY env var overrides backoff for all retries (set to 0 in tests to skip sleep).

---

Closes #159

Post-script verification

• Branch is not main/master (agent/159-retry-post-review)
• Secret scan passed (gitleaks — 51561b23f669baf5e6ad027c5de39b0fdb199ab9..HEAD)
• Pre-commit hooks passed (authoritative run on runner)
• Tests ran inside sandbox

[rh-hemartin]

/fs-fix rebase and resolve the conficts.

[fullsend-ai-coder]

🤖 Finished Fix · ✅ Success · Started 11:31 AM UTC · Completed 11:37 AM UTC
Commit: 7d0bfa9 · View workflow run →

[fullsend-ai-coder]

🔧 Fix agent — iteration 1 (human-triggered)

Rebased onto origin/main, resolved conflict in post-review-test.sh by merging both draft PR tests and retry tests, and updated degraded-mode fallback to include PR_IS_DRAFT check for consistency with main's outcome-label logic. All 58 tests pass.

Fixed (1):

1. rebase and resolve conflicts: Rebased branch onto origin/main (57 commits behind). Resolved merge conflict in scripts/post-review-test.sh by keeping both the draft PR / no-op label cycle tests (from main) and the retry logic tests (from the branch). Updated the degraded-mode fallback in post-review.sh to account for PR_IS_DRAFT, keeping it in sync with the main outcome-label block.

Tests: passed

Decision points

• Updated degraded-mode fallback label logic to include PR_IS_DRAFT check (alternatives: Leave fallback without draft awareness, Add draft check to match main outcome-label block; rationale: The fallback comment says 'Label logic mirrors the outcome-label block below — keep in sync'. Main added PR_IS_DRAFT awareness to the outcome-label block; the fallback must match to prevent draft PRs from receiving ready-for-merge via the degraded path.)

_{Updated by fullsend fix agent}

[rh-hemartin]

/fs-review

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 6:31 AM UTC · Completed 6:48 AM UTC
Commit: 7ada4e0 · View workflow run →

[fullsend-ai-review]

Review

Findings

Medium

• [correctness-bug] scripts/post-review.sh — The degraded-mode fallback applies outcome labels after retry exhaustion without verifying whether fullsend post-review successfully posted the review comment to GitHub. The code comment acknowledges this ambiguity ("the review comment may have been posted"), but if all retries failed before posting anything, applying ready-for-merge would misrepresent the PR state. Mitigated by exit 1 ensuring CI reports failure regardless.
  Remediation: If fullsend post-review can emit a distinct exit code for "comment posted, formal review failed," use that to gate the fallback. Otherwise, document the assumption explicitly in the comment block.

• [logic-error] scripts/post-review.sh — The degraded-mode fallback for the reject action applies the rejected label but does not close the PR, whereas the normal outcome-label path calls gh pr close with a comment before applying the label. The comment says "Label logic mirrors the outcome-label block below — keep in sync" but the close action is missing. In a degraded-mode reject scenario, the PR would be left open with a rejected label.
  Remediation: Either add gh pr close to the fallback's reject branch to mirror the normal path, or update the comment to explicitly note that closing is intentionally omitted in degraded mode as a conservative measure.

• [protected-path] scripts/post-review.sh, scripts/post-review-test.sh — This PR modifies files under scripts/, which is a protected path. The PR links to issue Post-review.sh should retry on transient GitHub API errors when submitting formal PR reviews #159 and provides clear rationale for the changes. Human approval is always required for protected-path changes, regardless of review agent findings.

Low

• [scope-creep] scripts/post-review.sh — The fallback applies labels for all actions (approve, comment, reject), but issue Post-review.sh should retry on transient GitHub API errors when submitting formal PR reviews #159 only specifically authorized fallback for the approve→ready-for-merge case. The extension to other actions is a reasonable engineering decision but exceeds the issue's explicit scope.

• [missing-requirement] scripts/post-review.sh — Issue Post-review.sh should retry on transient GitHub API errors when submitting formal PR reviews #159 requirement (4) asks for HTTP status code and response body in error messages, but the retry loop only logs the exit code. This is partially limited by fullsend post-review not exposing HTTP-level details.

• [code-duplication] scripts/post-review.sh — The fallback duplicates the outcome-label logic from the normal path. The comment acknowledges this ("keep in sync"), but a shared function could reduce maintenance burden and prevent the divergence noted in the [logic-error] finding above.

• [minor-behavioral-difference] scripts/post-review.sh — The fallback removes all three outcome labels (including the one about to be re-added) before applying the new one, creating an unnecessary remove+add cycle on the PR timeline. The normal path skips removing the target label to avoid this.

• [injection] scripts/post-review.sh — The ::warning:: command in the fallback interpolates ${ACTION} without sanitization for :: sequences or encoded newlines. The existing label-actions code (lines 264-269) explicitly sanitizes LA_ACTION for this exact pattern — the same defense-in-depth should apply here.

• [missing-variable-documentation] docs/review.md — New environment variable POST_REVIEW_RETRY_DELAY is not documented in the Variables section alongside REVIEW_FINDING_SEVERITY_THRESHOLD.

---

Labels: PR modifies review pipeline post-review scripts