post-review.sh fails when GitHub API returns 0 changed_files for open PRs

[fullsend-ai-retro]

What happened

Review agent was dispatched for PR #753 (a Pipfile.lock-only dependency update) on 2026-06-09 at 19:29 UTC (run 27230566881). The review agent itself completed successfully (exit code 0, valid schema output, ~39 seconds). The post-review script (scripts/post-review.sh) then failed with: Failed to fetch PR files or PR has no changed files — refusing to approve. The same failure recurred on run 27230989001. The PR was still open at the time (merged ~24 minutes later at 19:53 UTC). The GitHub API's changed_files summary field returned 0, triggering the safety guard. This made the entire review workflow report as failed despite the agent producing correct output.

What could go better

The post-review script uses a PR-level summary field (changed_files) to verify that the PR has files before approving. This field can return 0 in edge cases — likely when a merge commit temporarily makes the diff against the base branch empty (the PR branch had just merged main into it at 19:29 UTC). The safety guard is reasonable (preventing blind approvals) but its implementation is brittle. Confidence: high that this is a GitHub API data race — the timing (merge commit pushed at the same time as the review dispatch) strongly correlates. Two out of three review failures in the last 20 runs hit this exact error, suggesting it's not extremely rare.

Proposed change

In scripts/post-review.sh, replace the changed_files summary field check with a paginated call to the /pulls/{pull_number}/files endpoint (e.g., gh api repos/{owner}/{repo}/pulls/{pr}/files --paginate --jq 'length'). The /files endpoint enumerates actual file diffs and is more reliable than the summary field. If the /files endpoint also returns 0 files, add a short retry (e.g., 1 retry after 10 seconds) before failing, to handle transient API data races after merge commits are pushed. Keep the existing safety guard behavior (refuse to approve if truly no files) but make the file enumeration more robust.

Validation criteria

The next 10 review agent runs on PRs that have just received merge commits (rebase/merge from base branch) should not fail with 'Failed to fetch PR files'. Specifically, review runs on lock-file-only bot PRs in konflux-ci/tools should complete without post-review failures.

---

Generated by retro agent from konflux-ci/tools#753

[fullsend-ai-triage]

🤖 Finished Triage · ✅ Success · Started 8:03 PM UTC · Completed 8:05 PM UTC
Commit: 27b1874 · View workflow run →

[fullsend-ai-triage]

Triage Summary

Severity: medium · Category: bug

Problem

When the review agent produces an approve verdict, post-review.sh fetches the PR's changed files via gh pr view --json files (line 90) to run protected-path checks. If the GitHub API returns an empty files list — observed during merge-commit data races — the safety guard on line 91-94 triggers and the script exits with an error, failing the entire review workflow despite correct agent output.

Root Cause Hypothesis

The gh pr view --json files call can transiently return an empty list when a merge commit is pushed to the PR branch at approximately the same time as the review dispatch. Two out of three recent review failures hit this exact pattern, correlating with merge commits pushed at the time of dispatch.

Recommended Fix

In post-review.sh (lines 88-94):

1. Replace gh pr view --json files with a paginated REST API call: gh api repos/${REPO_FULL_NAME}/pulls/${PR_NUMBER}/files --paginate
2. Add a single retry with a 10-second delay if the endpoint returns 0 files
3. Keep the existing safety guard (refuse to approve if truly no files after retry)

Proposed Test Case

# In post-review-test.sh:
test_approve_retries_on_empty_files() {
  # Mock gh api to return empty on first call, non-empty on second
  # Assert: script proceeds to protected-path check
}

test_approve_fails_after_retry_exhausted() {
  # Mock gh api to return empty on both calls  
  # Assert: script exits with 'refusing to approve' error
}

Validation

The next 10 review agent runs on PRs that have just received merge commits should not fail with the 'Failed to fetch PR files' error.

---

Labels: Bug in the review agent post-script affecting the dispatch component, medium priority based on frequency (2 of 3 recent failures).

[fullsend-ai-prioritize]

RICE Priority Score: 2.7

Score breakdown

Dimension	Score	Reasoning
Reach	1.5	This affects all repos using the fullsend review agent whenever a merge commit coincides with review dispatch. Two out of three recent review failures hit this exact error, indicating it impacts multiple teams/repos across the platform, not just a single user.
Impact	1	The failure causes the review workflow to report as failed despite correct agent output, requiring manual re-runs. A workaround exists (re-trigger the review), so it's noticeable friction but not blocking — the review agent itself works correctly.
Confidence	0.9	Very high confidence. The issue is extremely well-described with clear root cause analysis, specific log references, correlated timing evidence, and a concrete proposed fix. The triage agent confirmed the diagnosis.
Effort	0.5	Simple, well-scoped change in a single file (post-review.sh, lines 88-94). Replace one API call with a paginated REST endpoint and add a retry loop. No architectural changes, no cross-component impact.

Formula: (1.5 x 1 x 0.9) / 0.5 = 2.7

[shairevivo]

The root cause is in the agents repo, not this one: post-review.sh refuses to approve when forge_get_pr_files returns an empty changed-files list, but that list can come back empty transiently (GitHub hasn't finished computing the diff right after a merge-commit update), so a genuinely non-empty PR gets failed.

Fix opened at fullsend-ai/agents#1196:

• forge_get_pr_files (GitHub) reads the paginated /pulls/{n}/files REST endpoint instead of the async-populated gh pr view --json files summary field.
• The empty-result guard retries once before refusing to approve — recovers from the transient race, still refuses on a genuinely empty result. Forge-agnostic (covers GitLab too).
• Added integration tests + regenerated bundles.