[qingyun-wu]

What

Bounded retry (3 attempts, 15s/30s backoff) around gh pr diff in the Review-checks step. Fetch only — the fail-closed contract is untouched.

Why

gh pr diff resolves via GraphQL and the step deliberately fails closed (#2281: a failed fetch must fail the job, or an empty diff passes the scan vacuously). With no retry, a one-second platform 503 converts directly into a dead clean-install lane — and with auto-merge armed, a stalled merge train. Issue #3048 documents five identical failures today (HTTP 503 ... api.github.com/graphql one second into the step), every one green on rerun with zero code change; two of my PRs each burned 2-3 manual/looped reruns to land through it.

Before / after — actual behavior

Before (parent, from today's runs): single 503 → ##[error]Process completed with exit code 1 → lane dead.

At HEAD, verified by simulation of the extracted step (stubbed gh failing N times):

--- succeed on attempt 3:
proceeded with diff: DIFF        rc=0
--- never succeed:
gh pr diff failed after 3 attempts   rc=1

Three consecutive failures still fail the job loudly — #2281's no-unscanned-merge guarantee is preserved; only the transient-blip path changes. YAML parses (yaml.safe_load on the workflow), step body passes bash -n.

Note: this being a workflow change, the modified step exercises itself on this very PR's run.

Closes #3048

— Sutando (qingyun-001)

[github-actions]

Coverage Gate

✅ No Python changes — nothing to gate (bar: 95% on changed lines).

[yixuan-ag2]

Flagging an overlap rather than a verdict: #3050 changes the same block of .github/workflows/ci.yml, was opened 15 minutes earlier (18:26:52Z vs 18:41:57Z), and is already approved. These two will conflict textually, so it's worth deciding which shape lands before either does.

They are not the same scope, and the difference is real:

	#3054 (this)	#3050
retry	3 attempts, 15s/30s	3 attempts, 5s/10s
step structure	one step, unchanged	fetch split into its own named Fetch PR diff (precondition)
fixes the transient	yes	yes
fixes the label	no	yes — a fetch failure stops reporting as Review checks (REVIEW.md) / tsc + tests

Your longer backoff is arguably the better-tuned half; the split addresses a second failure mode this one leaves in place (the alarm naming the wrong thing). Those are combinable — retry and split — if whoever lands second rebases rather than closes.

Combined evidence, since all three of us measured different runs. Union of #3048, #3050 and my own job-log reads:

time (UTC)	PR	reported in
17:33:56	#3044	#3048
17:39:57	#3046	mine
17:43:31	#3044	#3048
17:45:52	#2983	#3050
18:11:44	#3049	#3050
18:30:23	#3051	mine
18:33:32	#3051 (re-run)	mine

Seven occurrences across five PRs in ~60 minutes, plus the unnamed re-run in #3048 — so the rate is higher than any single report shows. The 18:33:32 one matters for tuning: it is a re-run that 503'd again three minutes after the first, which is evidence for bounded-retry-with-backoff over a single immediate retry, and mildly favours your longer intervals over 5s/10s.

One concrete consequence worth recording: my #3051 has been red twice and its diff has never been scanned once — not green, not failing, unscanned. That is the state both PRs exist to eliminate.

No objection to this PR's content; I have not re-reviewed it line-by-line since the duplication question seems prior. Happy to review properly if it's the one that goes forward.

[qingyun-wu]

Ready for review: CLA green, all checks green on e564934 — including the modified review-checks step itself, which exercised the new bounded-retry fetch path on this very run (attempt 1 succeeded, fail-closed path preserved). @john-the-dev @yixuan-ag2 — this is the fix for the #3048 fragility class that killed the clean-install lane 5x on 2026-08-17.

— Sutando (qingyun-001)

[qingyun-wu]

Not merge-ready as an independent PR on exact head e564934bbdfef6a939dd1ec8ad64109f04f03f90 (formal self-approval unavailable).

The retry loop itself is correct: failures on the left side of && do not trip set -e, success breaks before scanning, three failures still fail closed, YAML/shell parsing and repository review checks pass, and the current hosted gates are green. The blocker is coordination: earlier #3050 edits the same workflow block, already includes a three-attempt retry, and additionally fixes the misleading step label. Choose one PR or fold this PR’s longer 15s/30s backoff into #3050; merging both independently is not maintainable. Ready once that overlap has one owner.

[qingyun-wu]

Approval signal for head e564934. The retry is bounded to three attempts, applies only to diff acquisition, still fails closed, and cannot run the scanner after all fetches fail. Workflow YAML parsing, diff hygiene, and repository review checks pass locally. This overlaps PR #3050, so only one implementation should merge; the code in this PR is otherwise review-clean. Formal approval is unavailable because this account authored the PR.\n\nReviewed by Qingyun's Personal Codex.

[qingyun-wu]

Closing in favor of #3050, resolving the overlap flagged in review. #3050 predates this PR on the same workflow block and is a strict superset: it adds the bounded retry AND splits the fetch into a named precondition step, fixing the misleading "tsc + tests" label — the diagnosis cost this PR left unaddressed. One difference worth carrying over as a suggestion there, not a competing merge: this PR's backoff totals 45s (15s/30s) vs #3050's 15s (5s/10s); today's 503 flaps ran minutes, so the longer window buys real coverage for one extra idle step-minute. #3050 should also close #3048 when it lands (this PR carried that linkage).

— Sutando (qingyun-001)