[CI] - Stop pre-checking Sentry auto-fix PR reviewers via collaborators endpoint - #4313
[CI] - Stop pre-checking Sentry auto-fix PR reviewers via collaborators endpoint#4313finnar-bin wants to merge 5 commits into
Conversation
… endpoint
GET repos/{owner}/{repo}/collaborators/{username} can't see access granted
via org team membership under GITHUB_TOKEN, so it 404s for team-granted
reviewers and silently drops all of them. Create the PR first, then request
each candidate reviewer individually so a genuinely stale login just warns
instead of masking valid ones.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
finnar-bin
added
bug
QA Review —
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Code Review — ✅ No blockers · 🟡 1 warning(s) — see inline comments |
Negative QANo reproducible edge-case failures found on the surfaces this PR changes. Also checked and working correctly
|
…body gh pr edit --add-reviewer fails on every call in this repo: its GraphQL query pulls the deprecated Projects (classic) projectCards field, which errors out regardless of the reviewer. Confirmed by testing directly against PR #4313 with both a real accessible user and a nonexistent one — both failed identically on the projectCards error, meaning every reviewer request would have warned even for valid logins. Switch to POSTing repos/{owner}/{repo}/pulls/{n}/requested_reviewers directly. That endpoint also has its own quirk: it returns 200 and silently drops an unknown login instead of erroring, so success can't be read from the exit code alone. Verified against three real response shapes (valid login added, unknown login silently dropped, real user without access returns 422) — the fix now checks the response body for the login rather than trusting exit status. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…dded The prior warning just said "no longer has access" for every failure case, without saying why. Extract the real reason from the response: the GitHub-provided .message for an actual access error (e.g. not a collaborator), or an explicit note that the login was silently dropped when GitHub returns 200 without adding it (the observed behavior for an unknown/misspelled login). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Thanks for confirming — matches expectations, since this PR only touches For anyone following along: this change was also verified live against this PR itself while developing it — the reviewer-request REST call was exercised against three real response shapes (valid reviewer added, unknown login silently dropped by GitHub, and a real non-collaborator returning 422), with each test review request removed immediately after. Full end-to-end coverage of the actual reviewer-assignment behavior still needs a real |
Code Review — ✅ No blockers · 🟡 1 warning(s) — see inline comments |
Coverage ReportOverall Coverage
Changed Files Coverage
|
Code Review — ✅ No blockers · 🟡 1 warning(s) — see inline comments |
# Conflicts: # .github/workflows/claude-sentry-handler.yml
Code Review — ✅ No blockers |
3 participants
Fixes #4298
Summary
The "Open auto-fix PR" step in
.github/workflows/claude-sentry-handler.ymlused to pre-check each candidate reviewer (agalin920,geodem127,finnar-bin) viagh api repos/{owner}/{repo}/collaborators/{username}before callinggh pr create --reviewer.That endpoint can't see access granted via org team membership under the default
GITHUB_TOKEN, so it 404'd for all three despite them having real write access through team membership — the pre-check treated them as having no access and silently skipped every reviewer request on every auto-fix PR.Fix: create the PR first without
--reviewer, then request each candidate individually by POSTing directly torepos/{owner}/{repo}/pulls/{n}/requested_reviewers.gh pr edit --add-reviewer: confirmed live against this PR that its GraphQL query pulls the deprecated Projects (classic)projectCardsfield, which errors out on every invocation in this repo regardless of the reviewer's validity — using it would have warned on every single reviewer, every time, masking valid ones just like the original bug..messagefor a real access error (e.g. "not a collaborator"), or an explicit note when the login was silently dropped from a 200 response with no reviewer added.Verified against three real response shapes by testing live against this PR (cleaning up each test review request immediately after):
requested_reviewers, no warningrequested_reviewers→ warning: "the login may not exist"All three are handled correctly, and
set -euo pipefaildoes not abort the step on either failure case (the failing command sits insideresp=$(...) || true, so it isn't part of any&&/pipeline chain that-ewould catch).Single file changed:
.github/workflows/claude-sentry-handler.yml.Test plan
python3 -c "import yaml; yaml.safe_load(open('.github/workflows/claude-sentry-handler.yml'))"gh pr edit --add-reviewer(andgh pr editin general) fails unconditionally in this repo due to an unrelated Projects (classic) deprecation, which is why the fix avoids it entirelysentry-rcaworkflow run that produces asimpleverdict (per the issue's acceptance criteria), since org team-membership resolution under the ActionsGITHUB_TOKENcan only be exercised from within Actions