Skip to content

Commit 3b5eac0

Browse files
rdimitrovclaude
andcommitted
Assign reviewers per-user instead of pre-filtering by collaborator API
Replaces the upfront `gh api repos/X/collaborators/Y` filter with per-user `gh pr edit --add-reviewer` attempts. Routes GitHub's own rejection (422) into the mention list instead of relying on our filter to pre-compute the split. Why: on PR #759 the collaborator check returned 404 for Stacklok employees who ARE collaborators via the `stackers` team (push permission on docs-website) -- `ChrisJBurns`, `jhrozek`, `reyortiz3`, `tgrunnagle`. Only `rdimitrov` passed, so four reviewers silently vanished. A local check with a PAT that has read:org confirms all five are collaborators; the discrepancy appears to be GITHUB_TOKEN treating team-based access differently at the collaborator endpoint, though we haven't pinned the exact rule. The authoritative answer is "will GitHub accept this person as a reviewer right now" -- asking that question directly (via the add-reviewer API) avoids the filter being wrong. Each attempt is independent, so one rejection doesn't kill the batch. The separate "Add reviewers" step is now redundant -- assignments happen inline during candidate iteration. Dropped. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent bf3fe81 commit 3b5eac0

1 file changed

Lines changed: 35 additions & 31 deletions

File tree

.github/workflows/upstream-release-docs.yml

Lines changed: 35 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -391,13 +391,14 @@ jobs:
391391
id: pre_skill
392392
run: echo "sha=$(git rev-parse HEAD)" >> "$GITHUB_OUTPUT"
393393

394-
- name: Extract reviewers and contributors from release compare
394+
- name: Assign reviewers and prepare contributor mentions
395395
id: reviewers
396396
env:
397397
REPO: ${{ steps.detect.outputs.repo }}
398398
PREV: ${{ steps.detect.outputs.prev_tag }}
399399
NEW: ${{ steps.detect.outputs.new_tag }}
400400
REVIEW_REPO: ${{ github.repository }}
401+
PR_NUMBER: ${{ steps.eff.outputs.number }}
401402
run: |
402403
# Get non-bot commit authors in the release range.
403404
if COMPARE=$(gh api "repos/$REPO/compare/$PREV...$NEW" \
@@ -412,43 +413,53 @@ jobs:
412413
CANDIDATES=$(echo "$COMPARE" |
413414
grep -Ev '(\[bot\]$|^github-actions|^stacklokbot$|^dependabot|^renovate|^copilot)' || true)
414415
415-
# Split candidates into two groups:
416-
# - ASSIGN_LIST: collaborators on this repo -- safe to pass
417-
# to `gh pr edit --add-reviewer`. GitHub rejects the
418-
# entire call with 422 if any name in the list isn't a
419-
# collaborator, so the filter is mandatory to avoid
420-
# dropping valid reviewers alongside invalid ones.
421-
# - MENTION_LIST: everyone else. Upstream contributors
422-
# (often Stacklok employees with private org membership
423-
# we can't detect via GITHUB_TOKEN, which lacks
424-
# read:org) still deserve a ping so they see the PR
425-
# documenting their work. @-mentioning in the PR body
426-
# handles that without needing reviewer-assignment
427-
# rights.
428-
# Future: a PAT with read:org could let us also check
429-
# membership in the `stackers` team and promote those to
430-
# ASSIGN_LIST. Deferred -- not wiring a secret in now.
416+
# Attempt to assign each candidate as a reviewer individually,
417+
# rather than filtering upfront and batching. Rationale:
418+
# - `gh pr edit --add-reviewer "a,b,c"` is atomic. A single
419+
# 422 on any name aborts the whole call, dropping valid
420+
# names alongside invalid ones.
421+
# - `gh api repos/X/collaborators/Y` as a pre-filter is
422+
# unreliable from a GITHUB_TOKEN in Actions: on PR #759
423+
# the check returned 404 for Stacklok employees who ARE
424+
# collaborators via the `stackers` team (push perm on
425+
# this repo), and only `rdimitrov` slipped through. We
426+
# suspect the collaborator endpoint treats team-based
427+
# access differently for GITHUB_TOKEN vs PATs with
428+
# read:org, but haven't nailed down the exact rule.
429+
# Per-user attempts sidestep both issues: the authoritative
430+
# answer is "does GitHub accept this as a reviewer right now"
431+
# and we ask the API that question directly.
432+
#
433+
# Cap attempts at 5 to avoid review fatigue on big releases.
434+
TRIED=0
431435
ASSIGN_LIST=""
432436
MENTION_LIST=""
433437
while IFS= read -r login; do
434438
[ -z "$login" ] && continue
435-
if gh api "repos/$REVIEW_REPO/collaborators/$login" --silent 2>/dev/null; then
439+
if [ "$TRIED" -ge 5 ]; then
440+
# Over the review-fatigue cap -- mention any additional
441+
# contributors instead of trying to assign them.
442+
MENTION_LIST="${MENTION_LIST:+$MENTION_LIST }@$login"
443+
continue
444+
fi
445+
TRIED=$((TRIED + 1))
446+
if gh pr edit "$PR_NUMBER" --add-reviewer "$login" 2>/dev/null; then
436447
ASSIGN_LIST="${ASSIGN_LIST:+$ASSIGN_LIST,}$login"
448+
echo "Assigned: $login"
437449
else
438450
MENTION_LIST="${MENTION_LIST:+$MENTION_LIST }@$login"
451+
echo "Mention (assignment rejected by GitHub): $login"
439452
fi
440453
done <<< "$CANDIDATES"
441454
442-
# Cap auto-assignments at 5 to avoid review fatigue. Mentions
443-
# aren't capped -- they're cheap and the block is a single
444-
# PR-body notification.
445-
ASSIGN_LIST=$(echo "$ASSIGN_LIST" | tr ',' '\n' | head -5 | paste -sd, -)
446-
455+
# Exposed for diagnostic visibility in the PR body (e.g.,
456+
# "Auto-assigned: @alice @bob") and for the next workflow_
457+
# dispatch retry to know what was attempted.
447458
echo "list=$ASSIGN_LIST" >> "$GITHUB_OUTPUT"
448459
{
449460
echo "mention_block<<MENTION_EOF"
450461
if [ -n "$MENTION_LIST" ]; then
451-
echo "Release contributors who aren't collaborators on this repo and so couldn't be auto-assigned as reviewers. Mentioning them so they see the PR documenting their work:"
462+
echo "Release contributors we couldn't auto-assign as reviewers (review fatigue cap or GitHub rejected the assignment). Mentioning them so they see the PR documenting their work:"
452463
echo ""
453464
echo "$MENTION_LIST"
454465
fi
@@ -896,13 +907,6 @@ jobs:
896907
897908
gh pr edit "$PR_NUMBER" --body-file /tmp/pr-body.md
898909
899-
- name: Add reviewers
900-
if: always() && steps.reviewers.outputs.list != ''
901-
env:
902-
PR_NUMBER: ${{ steps.eff.outputs.number }}
903-
REVIEWERS: ${{ steps.reviewers.outputs.list }}
904-
run: gh pr edit "$PR_NUMBER" --add-reviewer "$REVIEWERS"
905-
906910
- name: Comment on augmentation failure
907911
# Runs only when a preceding step failed. Comments a retry
908912
# pointer on the PR so a human can see the run URL.

0 commit comments

Comments
 (0)