fix: treat unverifiable submitter access as warning instead of error#452
Conversation
The collaborator API requires admin access on the target repo, which the Actions token never has for external repos. This caused all leaderboard submissions for org-owned repos to fail CI with a false "no access" error. Align the GitHub path with the non-GitHub fallback: warn maintainers to verify manually rather than blocking the PR. Fixes CI for #448. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe leaderboard GitHub Actions workflow changes the submitter access check to call ChangesLeaderboard Submission Access Handling
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/leaderboard.yml:
- Around line 158-165: The script incorrectly treats HTTP_CODE as an HTTP status
and assumes a 403; rename the variable (e.g., HTTP_CODE -> API_EXIT or
EXIT_CODE) to reflect it holds gh api's exit code, and update the conditional
logic/messages around the gh api "/repos/$ORG_REPO/collaborators/$SUBMITTER"
call and the else branch to not assert a specific status; instead emit a generic
warning that the API check failed (include EXIT_CODE value) and that manual
verification is required. Ensure you update references to HTTP_CODE everywhere
in this block (the gh api invocation, subsequent if/elif/else checks and the
echo warning lines) and keep the existing owner check using gh api
"/repos/$ORG_REPO" -q '.owner.login' unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 4cc2d39b-2c15-4d0a-a8b8-d69a83b9507b
📒 Files selected for processing (1)
.github/workflows/leaderboard.yml
📈 Test Coverage Report
Coverage calculated from unit tests only |
jwm4
left a comment
jwm4
left a comment
There was a problem hiding this comment.
Review
Overall: Looks good, with a couple of minor nits.
The problem is well-understood: the /repos/{owner}/{repo}/collaborators/{user} endpoint requires admin access, which GITHUB_TOKEN never has for external repos. This means every leaderboard submission for org-owned repos fails CI with a false negative. Converting the hard error to a warning for manual review is the right fix, and it mirrors the existing non-GitHub fallback at line 168.
Nits
-
Misleading variable name.
HTTP_CODEcaptures the process exit code ($?), not an HTTP status code. Calling itEXIT_CODEorRCwould be more accurate. -
Warning message hardcodes "403". The
elsebranch triggers for any API failure (network errors, 404, 500, etc.), but the message says(API returned 403). Something like(API check failed)would be more accurate. -
Unnecessary refactor of the
ifcondition. The originalif gh api ... 2>/dev/null; thenalready checks the exit code. You can just add--silentinline and only change theelsebranch:if gh api "/repos/$ORG_REPO/collaborators/$SUBMITTER" --silent 2>/dev/null; then echo "✅ $SUBMITTER is a collaborator on $ORG_REPO" elif ... ... else echo "::warning::Cannot verify submitter access for $ORG_REPO (API check failed). Manual review required." echo "⚠️ Submitter access must be verified manually by maintainers." fi
This is a smaller diff, easier to read, and avoids the misleading
HTTP_CODEvariable entirely.
What looks good
- Correctly scoped fix: only changes the failure path, leaves the success and owner-check paths untouched
- The
elifbranch (owner check via/repos/{owner}/{repo}) uses a public endpoint, so it will still work for public repos even when the collaborators check 403s - PR description clearly explains the root cause and links to #448
- Aligns behavior: GitHub path now matches the non-GitHub fallback pattern
Verdict
The logic is correct and the fix addresses the real problem. The three nits above are all cosmetic. I'd suggest the simplified approach from nit #3 since it shrinks the diff and avoids the naming issue, but the current code works fine functionally.
-- Bill Murdock with assistance from Claude Code
Drop the intermediate HTTP_CODE variable and restore the original inline `if gh api ...; then` pattern. Replace the hardcoded "403" in the warning message with the generic "API check failed". Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/leaderboard.yml:
- Around line 158-164: The workflow currently treats any non-success outcome as
"unverifiable" and warns; change the logic to three outcomes: if gh api
"/repos/$ORG_REPO/collaborators/$SUBMITTER" succeeds -> collaborator ✅; else if
gh api "/repos/$ORG_REPO" succeeds (repo exists) and the owner check (gh api
"/repos/$ORG_REPO" -q '.owner.login' == "$SUBMITTER") is false -> definitive
no-access ❌ (echo a failure message and exit 1); otherwise (any gh api call for
the repo fails) -> API-unverifiable ⚠️ (emit the warning but do not fail). Use
the existing gh api calls ("/repos/$ORG_REPO/collaborators/$SUBMITTER" and
"/repos/$ORG_REPO") and the owner check to distinguish repo-exists vs API-error
cases and ensure only the definitive no-access path fails the job.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: d1823fc0-bcca-48aa-849b-a27a0b861ac8
📒 Files selected for processing (1)
.github/workflows/leaderboard.yml
| if gh api "/repos/$ORG_REPO/collaborators/$SUBMITTER" --silent 2>/dev/null; then | ||
| echo "✅ $SUBMITTER is a collaborator on $ORG_REPO" | ||
| elif [ "$(gh api "/repos/$ORG_REPO" -q '.owner.login')" == "$SUBMITTER" ]; then | ||
| echo "✅ $SUBMITTER is the owner of $ORG_REPO" | ||
| else | ||
| echo "::error::$SUBMITTER does not have commit access to $ORG_REPO" | ||
| exit 1 | ||
| echo "::warning::Cannot verify submitter access for $ORG_REPO (API check failed). Manual review required." | ||
| echo "⚠️ Submitter access must be verified manually by maintainers." |
There was a problem hiding this comment.
Scope warning fallback to unverifiable cases only
This branch now warns on all non-success outcomes, so a submitter without verified access is no longer blocked. That broadens behavior beyond “unverifiable only” and weakens the gate.
Use a three-way outcome: collaborator/owner ✅, definitive no-access ❌ (fail), API-unverifiable
Suggested patch
- if gh api "/repos/$ORG_REPO/collaborators/$SUBMITTER" --silent 2>/dev/null; then
+ COLLAB_STATUS="$(gh api -i "/repos/$ORG_REPO/collaborators/$SUBMITTER" 2>/dev/null || true)"
+ COLLAB_HTTP="$(printf '%s\n' "$COLLAB_STATUS" | awk 'NR==1 {print $2}')"
+ if [ "$COLLAB_HTTP" = "204" ]; then
echo "✅ $SUBMITTER is a collaborator on $ORG_REPO"
elif [ "$(gh api "/repos/$ORG_REPO" -q '.owner.login')" == "$SUBMITTER" ]; then
echo "✅ $SUBMITTER is the owner of $ORG_REPO"
+ elif [ "$COLLAB_HTTP" = "404" ]; then
+ echo "::error::$SUBMITTER does not appear to have access to $ORG_REPO."
+ exit 1
else
echo "::warning::Cannot verify submitter access for $ORG_REPO (API check failed). Manual review required."
echo "⚠️ Submitter access must be verified manually by maintainers."
fi🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/leaderboard.yml around lines 158 - 164, The workflow
currently treats any non-success outcome as "unverifiable" and warns; change the
logic to three outcomes: if gh api "/repos/$ORG_REPO/collaborators/$SUBMITTER"
succeeds -> collaborator ✅; else if gh api "/repos/$ORG_REPO" succeeds (repo
exists) and the owner check (gh api "/repos/$ORG_REPO" -q '.owner.login' ==
"$SUBMITTER") is false -> definitive no-access ❌ (echo a failure message and
exit 1); otherwise (any gh api call for the repo fails) -> API-unverifiable ⚠️
(emit the warning but do not fail). Use the existing gh api calls
("/repos/$ORG_REPO/collaborators/$SUBMITTER" and "/repos/$ORG_REPO") and the
owner check to distinguish repo-exists vs API-error cases and ensure only the
definitive no-access path fails the job.
|
🎉 This PR is included in version 2.39.2 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
1 participant
Summary
Fixes CI for #448.
Test plan
Generated with assistance from Claude Code by Bill Murdock.
Summary by CodeRabbit