Skip to content

ci: paginate the pending-release query and guard its lookups #88

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mogita merged 3 commits into from
Sep 17, 2026
+62 −13
Merged

ci: paginate the pending-release query and guard its lookups #88

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8128a9b
fix(ci): paginate the pending-release query and guard its lookups
mogita Sep 17, 2026
3b1f371
fix(ci): treat an unknown release state as pending, not as none
mogita Sep 17, 2026
d6a2bf1
ci: give the stuck-release comment an explicit repo
mogita Sep 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 62 additions & 13 deletions .github/workflows/release.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,10 @@ jobs:
release-pr:
name: Release PR
needs: detect
if: needs.detect.outputs.pending != 'true'
# Gate on the explicit value. If detect fails or is skipped the output is empty, and
# anything short of a definite "nothing pending" has to hold this job back, or it
# proposes a second Release PR on top of one that may still be untagged.
if: needs.detect.outputs.pending == 'false'
runs-on: ubuntu-latest
timeout-minutes: 5
permissions:
Expand Down Expand Up @@ -55,7 +58,9 @@ jobs:
timeout-minutes: 5
permissions:
contents: read
pull-requests: read
# write, not read: a stuck release is announced on its own Release PR, because
# nothing else reaches a person without them opening the run first.
pull-requests: write
issues: read
outputs:
pending: ${{ steps.find.outputs.pending }}
Expand All @@ -70,35 +75,79 @@ jobs:
run: |
pending=false
ready=false
lookup_failed=false

# Query the label directly. Listing closed PRs and filtering client-side loses
# a release that has slipped past the first page, which reads as "nothing to
# release" and passes. Filter before picking, too: a hotfix branch can hold
# its own pending release, and taking the newest label match would drop this
# branch's release on every run until the other one clears.
nums="$(gh api "repos/${GITHUB_REPOSITORY}/issues" \
-X GET -f state=closed -f labels='autorelease: pending' -f per_page=20 \
--jq '.[] | select(.pull_request != null) | .number')"
# Query the label directly, and page: release-please applies the label when it
# opens the Release PR, not when it merges, so every Release PR closed without
# merging keeps it forever and holds a slot in this listing. One page would
# eventually stop containing the genuinely pending release, which reads as
# "nothing to release" and passes. merged_at comes back in the listing, so
# filtering on it here keeps the per-PR lookups below to real candidates.
# --paginate makes this N requests, so guard it too, and remember that a failure
# here means "unknown", never "nothing to release".
if ! nums="$(gh api --paginate "repos/${GITHUB_REPOSITORY}/issues" \
-X GET -f state=closed -f labels='autorelease: pending' -f per_page=100 \
--jq '.[] | select(.pull_request.merged_at != null) | .number')"; then
echo "::warning::Could not list pending releases."
nums=""
lookup_failed=true
fi

# The base branch is not in that listing, so each candidate still needs a
# lookup: a hotfix branch can hold its own pending release, and taking the
# newest label match would drop this branch's release until the other clears.
num=""
sha=""
for n in $nums; do
sha="$(gh api "repos/${GITHUB_REPOSITORY}/pulls/${n}" \
--jq 'select(.merged_at != null and .base.ref == env.BASE) | .merge_commit_sha // empty')"
# Under `bash -e` an unguarded assignment from a non-2xx would abort the
# step, which would fail detect and skip release-pr with it.
if ! sha="$(gh api "repos/${GITHUB_REPOSITORY}/pulls/${n}" \
--jq 'select(.merged_at != null and .base.ref == env.BASE) | .merge_commit_sha // empty')"; then
echo "::warning::Could not read PR #${n}; skipping it."

@mogita mogita Sep 17, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

A failed lookup now reads as "nothing to release". release-please swaps the label for autorelease: tagged when it tags, so in steady state nums holds exactly one number: the pending release. One 502 on it warns, continues, and the loop ends with sha="", so the step prints "No pending release on master", sets pending=false, and release-pr opens a second Release PR on top of the untagged one while the tag job never runs. Aborting was wrong, but reading unknown as "nothing pending" is worse than aborting.

With two stuck candidates it also misreports: skipping the newest lets an older one match, so the "Release stuck" warning names the wrong PR and tells the operator to re-run the wrong sha.

Set lookup_failed=true here, and after the loop treat [ -z "$sha" ] && [ "$lookup_failed" = true ] as pending=true, so an unknown answer stands down instead of releasing.

@mogita mogita Sep 17, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Fixed in 3b1f371. lookup_failed is set on both failure paths, and the branch reads:

if [ -z "$sha" ] && [ "$lookup_failed" = true ]; then
  pending=true
  echo "::warning::Could not determine whether a release is pending on ${BASE}; standing down."
elif [ -z "$sha" ]; then
  echo "No pending release on ${BASE}."

Given how often this logic has been wrong, I stubbed gh and ran the branch:

scenario pending ready
nothing pending false false
pending at this commit true true
pending at another sha true false
listing call fails true false
candidate call fails true false

The misreporting case you raise, where skipping the newest lets an older candidate match and the warning names the wrong PR, is now covered too: any skipped candidate sets lookup_failed, so a wrong-PR match cannot be reported as a clean result.

sha=""
lookup_failed=true
continue
fi
if [ -n "$sha" ]; then
num="$n"
break
fi
done

if [ -z "$sha" ]; then
if [ -z "$sha" ] && [ "$lookup_failed" = true ]; then
# Unknown is not the same as nothing. Releasing on a guess is how a second
# Release PR lands on top of one that was never tagged.
pending=true
echo "::warning::Could not determine whether a release is pending on ${BASE}; standing down."
elif [ -z "$sha" ]; then
echo "No pending release on ${BASE}."
elif [ "$sha" != "$HEAD_SHA" ]; then
# Reached when an earlier release run failed after the Release PR merged.
# Tagging $sha here would tag a tree this run never tested, and failing
# would redden every later push, so stand down and say why.
pending=true
echo "::warning::Release PR #${num} is still pending at ${sha}, which is not this run's commit ${HEAD_SHA}. Re-run the workflow run for ${sha} to finish that release."
{
echo "### Release stuck"
echo
echo "Release PR #${num} merged at \`${sha}\` and was never tagged, so no Release PR will be opened or refreshed until it clears."
echo
echo "Re-run the \`Release\` run for \`${sha}\`. If that commit is genuinely broken, remove the \`autorelease: pending\` label from #${num} by hand and release forward."
} >> "$GITHUB_STEP_SUMMARY"

@mogita mogita Sep 17, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

The PR names the problem as "a stuck release stays stuck until somebody opens the run and reads the annotation", and a step summary is also only visible when somebody opens the run. The conclusion stays green and nothing notifies anyone.

gh pr comment "$num" on the stuck Release PR reaches its subscribers and is one line. Guard it with a marker check so every later push does not add another.

@mogita mogita Sep 17, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Fixed in 3b1f371. The stuck branch now comments on the Release PR itself, guarded by a per-sha marker so later pushes do not pile up:

marker="<!-- release-stuck:${sha} -->"
seen="$(gh api "repos/${GITHUB_REPOSITORY}/issues/${num}/comments" --paginate --jq '.[].body' || echo "")"
if ! printf '%s' "$seen" | grep -qF "$marker"; then
  ...
  gh pr comment "$num" --body-file "${RUNNER_TEMP}/release-stuck.md" || echo "::warning::Could not comment on #${num}."
fi

detect takes pull-requests: write for it, which is the one cost. The comment write is || echo, so it can never be what fails the job.

Keeping the step summary as well: it is free and it is where someone already in the run will look.

# A warning annotation and a step summary are both only visible to someone who
# already opened the run. Tell the Release PR's subscribers once per stuck sha.
marker="<!-- release-stuck:${sha} -->"
seen="$(gh api "repos/${GITHUB_REPOSITORY}/issues/${num}/comments" --paginate --jq '.[].body' || echo "")"
if ! printf '%s' "$seen" | grep -qF "$marker"; then
{
echo "$marker"
echo "This release is stuck: #${num} merged at \`${sha}\` and was never tagged, so no Release PR is opened or refreshed until it clears."
echo
echo "Re-run the \`Release\` run for \`${sha}\`. If that commit is genuinely broken, remove the \`autorelease: pending\` label here by hand and release forward."
} > "${RUNNER_TEMP}/release-stuck.md"
gh pr comment "$num" --repo "$GITHUB_REPOSITORY" --body-file "${RUNNER_TEMP}/release-stuck.md" \
|| echo "::warning::Could not comment on #${num}."
fi
else
pending=true
ready=true
Expand Down
Loading