Skip to content

Retry GitHub API calls, and make the writes safe to retry - #35

Merged
github-actions[bot] merged 1 commit into
mainfrom
fix/gh-api-retry
Aug 17, 2026
Merged

Retry GitHub API calls, and make the writes safe to retry#35
github-actions[bot] merged 1 commit into
mainfrom
fix/gh-api-retry

Conversation

@melbinjp

@melbinjp melbinjp commented Aug 17, 2026

Copy link
Copy Markdown
Owner

GitHub had an incident on 2026-08-17 that produced five separate red checks on this repo with nothing wrong in the code:

check failure
Enable auto-merge 503 Service Unavailable from gh pr merge
Jules Review 503 posting a comment
Clear Failure Report 503 from gh issue close, three attempts running
Fresh Install artifact upload ENOTFOUND (GitHub-side, not fixable here)
workflow_dispatch 503 twice before it was accepted

The one that actually costs something

Clear Failure Report closes the failure issue when CI goes green again, so when it fails the tracker keeps saying "currently broken" about something that is already fixed.

#28 is open right now for exactly that reason. The scheduled run passed after #34 and the close 503'd three times. An issue that says "broken" when it is not is precisely the signal that job exists to protect. The others are milder but not free: a red X that means "GitHub had a bad minute" trains people to stop reading red Xs.

Why not just wrap everything in a retry

gh issue create and gh issue comment are not idempotent. If the write actually succeeded and only the response was lost, a naive retry files a duplicate issue or double-comments.

So rather than assume a 503 never reached the backend, both loops re-derive state before every attempt: re-list the open issue, then decide comment-vs-create, or re-list and exit clean if it is already closed. Idempotent by construction rather than by assumption.

gh pr merge --auto needs none of that. It sets a flag on the PR, so arming it twice is the same as arming it once, and it is retried directly.

Verified, not asserted

Both run: blocks were extracted from the YAML and executed against a stub gh that simulates the outage, including the lost-response cases the design exists to handle:

outage then ok   list 503s twice, then closes #28, exit 0
lost response    close reports failure but had worked -> next pass finds nothing
                 open and exits 0.  No double close.
create lost      create reports failure but had worked -> next pass finds the
                 issue and comments.  No duplicate filed.
always down      five attempts, then exit 1 rather than a silent pass

Follows #34, which fixed the three-week-old Python 3.10 break. That one was our own code; this one is not, but it is our own code that decides how loudly someone else's bad minute shows up here.

GitHub had an incident on 2026-08-17. It produced five separate red checks on this
repo with nothing wrong in the code:

  Enable auto-merge      503 Service Unavailable from gh pr merge
  Jules Review           503 posting a comment
  Clear Failure Report   503 from gh issue close, three attempts running
  Fresh Install artifact ENOTFOUND uploading (GitHub-side, not fixable here)
  workflow_dispatch      503 twice before it was accepted

The one that actually costs something is Clear Failure Report. It closes the failure
issue when CI goes green again, so when it fails the tracker keeps saying "currently
broken" about something already fixed. #28 is open right now for exactly that reason:
the scheduled run passed and the close 503'd. That is the signal this job exists to
protect. The rest just train people to ignore a red X.

A blanket retry would be wrong for two of these. `gh issue create` and `gh issue
comment` are not idempotent, so retrying a lost response files a duplicate. Rather
than assume a 503 never reached the backend, both loops now RE-DERIVE state before
every attempt: re-list the open issue, then decide comment-vs-create; or re-list and
exit clean if it is already closed. Idempotent by construction.

`gh pr merge --auto` needs none of that. It sets a flag, so arming it twice is the
same as arming it once.

Verified against a stub gh that simulates the outage, all four paths:

  outage then ok  list 503s twice, then closes #28, exit 0
  lost response   close reports failure but had worked -> next pass finds nothing
                  open and exits 0. No double close.
  create lost     create reports failure but had worked -> next pass finds the
                  issue and comments. No duplicate filed.
  always down     five attempts, then exit 1 rather than a silent pass
@github-actions
github-actions Bot enabled auto-merge (squash) August 17, 2026 18:16
@github-actions

github-actions Bot commented Aug 17, 2026

Copy link
Copy Markdown

Jules Review

COVERAGE: 5b5e10c 2 files

Summary

This PR wraps GitHub API calls (gh) in our CI workflow files (auto-merge.yml and scheduled-ci.yml) in robust retry loops to handle transient API outages (like 503s). It cleverly ensures retries are structurally safe by re-deriving the issue state before each attempt rather than blindly retrying a create or close operation that might have actually succeeded on the backend before the connection dropped. The shell logic elegantly leverages if conditions to evaluate command success without tripping set -e, and preserves standard error for logging. The approach is sound, secure (using environment variables instead of script interpolation), and correctly addresses the problem described.

Findings

[WARN]

.github/workflows/scheduled-ci.yml, line 344
The PR description notes this approach avoids double-commenting, but re-listing before commenting does not strictly make gh issue comment idempotent. If gh issue comment succeeds but the response is lost, the next loop re-lists the issue (which remains open) and runs gh issue comment again, resulting in a duplicate comment anyway. This is still an improvement since it prevents duplicate issues, but it isn't fully idempotent for the comment path specifically.

[NIT]

.github/workflows/auto-merge.yml, line 64
.github/workflows/scheduled-ci.yml, line 355
.github/workflows/scheduled-ci.yml, line 397
The retry loops unconditionally sleep even on their final attempt before failing. On attempt=5, the script prints that it is retrying, sleeps for 50 seconds, and then immediately drops out of the loop and exits 1. Consider skipping the sleep if attempt -eq 5 to fail fast once retries are exhausted.

VERDICT: approve


This review never edits code or force-blocks a merge. No blocking issues were found, so this PR was auto-approved.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated approval: Jules found no blocking issues (verdict: approve). See the review comment above.

@github-actions
github-actions Bot merged commit ef85d2c into main Aug 17, 2026
22 of 23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant