Contributor fix#70
Conversation
…o wrong users Prioritize commit-search by email (most reliable) over direct username lookup. Restrict direct lookup to GitHub noreply emails only. Return null for unverifiable contributors instead of constructing URLs from guessed usernames. Also handle bare noreply format (username@users.noreply.github.com) without numeric prefix.
…-Remaining checks Add delay(ms) helper function and checkRateLimit() that reads X-RateLimit-Remaining header. If limit is exhausted, sleeps until X-RateLimit-Reset. Logs warning when remaining < 100. Adds 200ms delay between API calls to avoid burst limits. Fixes: CR-03 equivalent - no rate-limit handling for GitHub API calls
Add early-exit step that checks if the workflow was triggered by github-actions[bot]. When the bot pushes a README update, the push event would normally re-trigger the workflow recursively. This check breaks that loop by exiting with code 0 before any work is done. Fixes: CR-04 equivalent - recursive trigger risk when bot pushes to main
|
@Yuvraj-Sarathe is attempting to deploy a commit to the Rishi Bhardwaj's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughTightens contributor enrichment: adds rate-limit sleep/warning, stricter noreply username verification (returns null for unverified), filters nulls during seeding/updates, appends ChangesContributor enrichment and safety improvements
sequenceDiagram
participant Script as update-contributors.js
participant CommitSearch as GitHub Commit Search API
participant UserLookup as GitHub Users API
participant README as README.md
Script->>CommitSearch: search commits by author-email
CommitSearch-->>Script: commit results (checkRateLimit)
Script->>UserLookup: (noreply-only, after brief delay) GET /users/{guessedUsername}
UserLookup-->>Script: user profile (validated fields)
Script->>README: write only verified contributor objects (filter null)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/scripts/update-contributors.js:
- Around line 434-441: The current check reads the local README (freshContent)
after the rebase, so getExistingContributors() sees the rebased commit and
mistakenly treats trulyNew as already added; instead fetch or read the README
from origin/main and compare against that remote content. Replace the
fs.readFileSync(README_PATH) usage with a remote-read of origin/main (e.g., git
fetch origin then git show origin/main:README.md or equivalent) and pass that
content into getExistingContributors to compute freshExisting, keeping the rest
of the logic around trulyNew/stillNew unchanged.
- Around line 39-45: checkRateLimit() currently only sleeps and returns
undefined, so callers (the commit search and direct lookup that store responses
in commitSearch and direct) proceed to check `.ok` on the exhausted response and
skip retries; change the callers so that after calling await
checkRateLimit(resp) you reissue the same fetch when checkRateLimit indicates a
wait occurred (e.g., returns true) and replace commitSearch/direct with the new
fetch response before checking `.ok`. Concretely: in the commit-search path (the
code that assigns commitSearch) and in the direct-lookup path (the code that
assigns direct), preserve the original fetch arguments, call await
checkRateLimit(response) when rate-limited, then call fetch(...) again with the
same arguments and assign that new Response back to commitSearch or direct prior
to continuing.
In @.github/workflows/update-contributors.yml:
- Around line 16-21: The step named "Skip if triggered by bot" doesn't stop the
job because exit 0 only ends the step; move the condition to the job level by
adding an if at the job declaration that checks the actor (e.g., if:
github.actor != 'github-actions[bot]' && github.actor != 'github-actions') so
the entire job is skipped for bot triggers; remove the existing "Skip if
triggered by bot" step (or keep it for logging only) and ensure subsequent steps
(checkout, setup-node, run script) are under the job guarded by that job-level
if.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8eddee29-f783-422c-9a21-f02844390ca7
📒 Files selected for processing (2)
.github/scripts/update-contributors.js.github/workflows/update-contributors.yml
🔗 Related Issue
Closes #61
📝 Description of Changes
Tried somethings
🏷️ Proposed Labels
📂 Core Files Changed
.github\workflows\update-contributors.yml.github\scripts\update-contributors.js🤖 AI Assistance Declaration
Did you use an AI tool to write or assist with this code OR Pull Request?
Review Please
✅ The "I Swear I Didn't Break Anything" Pledge
Summary by CodeRabbit