-
Notifications
You must be signed in to change notification settings - Fork 19
Add GitHub Actions automation #189
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.
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
Conversation
WalkthroughAdds a new GitHub Actions workflow (.github/workflows/automation.yml) providing four automation jobs (bounty-tracking, update-leaderboard, welcome-contributor, notify-pr-merge) and three new JSON state files ( Changes
Sequence Diagram(s)sequenceDiagram
participant GH as GitHub
participant WF as Workflow
participant Py as Python script
participant Files as State files
participant Discord as Discord
GH->>WF: PR merged / schedule / dispatch
WF->>Py: run relevant job (bounty-tracking / leaderboard / welcome / notify)
Py->>Files: read state (`bounties_pending.json`, `contributors.json`, `payments_history.json`)
Py->>GH: query PRs or repo data via gh CLI
Py->>Py: parse bounties / aggregate stats / prepare messages
Py->>Files: write updates (state, reports, LEADERBOARD.md)
Py->>Discord: post message (if generated)
Py->>GH: commit changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🧹 Nitpick comments (4)
.github/workflows/automation.yml (4)
63-65: Potential data loss with PR list limit of 50.Line 63 uses
--limit 50when querying merged PRs. If the repository has more than 50 merged PRs since the last workflow run, older bounties will be skipped and never tracked. Consider:
- Increasing the limit or removing it (if API allows)
- Tracking the last run date and querying only PRs merged after that timestamp
- Adding pagination support
- cmd = ['gh', 'pr', 'list', '--repo', self.repo, '--state', 'merged', '--limit', '50', '--json', 'number,title,author,mergedAt,body,labels'] + cmd = ['gh', 'pr', 'list', '--repo', self.repo, '--state', 'merged', '--limit', '100', '--json', 'number,title,author,mergedAt,body,labels']At minimum, increase the limit or document the assumption about PR query frequency.
54-60: Bounty amount extraction is fragile and may miss valid formats.The regex patterns at lines 56 are specific but may not capture all bounty formats used in PRs. For robustness, consider:
- Documenting expected bounty format in contributing guidelines
- Adding logging to debug mismatches
- Testing patterns against real PR titles/bodies in the repo
Additionally, line 75 assigns a default of
$100when a bounty label exists but no amount is found. This may be surprising to contributors. Document the fallback behavior clearly.
155-160: git push with|| truesuppresses all errors silently.Lines 160 use
git push || true, which masks legitimate auth errors, permission issues, or network problems. The workflow will silently succeed even if changes weren't pushed to the repository. Consider:- git push || true + git pushIf you expect occasional transient push failures, use a proper retry mechanism or at least log the failure. Otherwise, remove the
|| trueto fail loudly on issues.
353-376: Contributor initialization overwrites existing data without merging.The Python code at lines 363–375 checks
if author not in contributorsbut does not merge or update existing fields if the author is already present. If a contributor updates their payment method or expertise later, this block won't update it. If future jobs try to write additional metadata (e.g., from a contributor profile), there's no merge logic:if author not in contributors: contributors[author] = { ... } # If author already exists, nothing happens — data isn't merged or updatedFor safety, consider:
- Explicitly defining all fields to be updated
- Using a merge function to handle new/existing records consistently
- Documenting the onboarding data structure clearly
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/automation.yml(1 hunks)bounties_pending.json(1 hunks)contributors.json(1 hunks)payments_history.json(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.8)
.github/workflows/automation.yml
15-15: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
20-20: the runner of "actions/setup-python@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
166-166: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
171-171: the runner of "actions/setup-python@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
289-289: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
308-308: the runner of "actions/github-script@v6" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🔇 Additional comments (5)
bounties_pending.json (1)
1-1: State file initialization looks good.The empty array correctly initializes the bounties storage structure for the workflow.
contributors.json (1)
1-1: State file initialization is correct.The empty object properly initializes the contributor records storage keyed by username.
payments_history.json (1)
1-1: State file initialization is appropriate.The empty array provides the correct structure for storing payment history records.
.github/workflows/automation.yml (2)
286-286: Verifypull_request.mergedcheck works withtypes: [closed]trigger.To run a workflow when a pull request merges, use the pull_request closed event type along with a conditional that checks the merged value of the event. However, line 3 defines
pull_request: types: [closed]globally, which means all closed PRs (merged or not) will trigger workflow jobs that checkgithub.event.pull_request.merged == true. This is correct at the job level but ensure that bothwelcome-contributorandnotify-pr-mergejobs properly filter for merged PRs only via their individualifconditions, and that the workflow doesn't inadvertently run expensive operations on every PR closure.The condition at line 286 appears correct, but verify that jobs aren't running on PR closures without merges.
15-15: Outdated GitHub Actions versions require updates.The following actions are flagged as outdated by actionlint and should be upgraded to their latest versions:
- Line 15:
actions/checkout@v3→ update toactions/checkout@v4or later- Line 20:
actions/setup-python@v4→ update toactions/setup-python@v5or later- Line 166:
actions/checkout@v3→ update toactions/checkout@v4or later- Line 171:
actions/setup-python@v4→ update toactions/setup-python@v5or later- Line 289:
actions/checkout@v3→ update toactions/checkout@v4or later- Line 308:
actions/github-script@v6→ update toactions/github-script@v7or laterApply these diffs to each location:
- uses: actions/checkout@v3 + uses: actions/checkout@v4- uses: actions/setup-python@v4 + uses: actions/setup-python@v5- uses: actions/github-script@v6 + uses: actions/github-script@v7Also applies to: 20-20, 166-166, 171-171, 289-289, 308-308
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (3)
.github/workflows/automation.yml (3)
165-165: CRITICAL: Invalid GitHub Actions context variable prevents update-leaderboard job from running.Line 165 uses
github.event.schedule == '0 12 * * 1', butgithub.event.scheduledoes not exist in GitHub Actions contexts. This condition is always false, so the job will never run—even when triggered by the Monday noon schedule (line 7).The correct way to check for a scheduled workflow is
github.event_name == 'schedule'. However, there is no standard context variable to distinguish which cron schedule triggered the workflow at runtime. Apply this diff:- if: github.event.schedule == '0 12 * * 1' || github.event_name == 'workflow_dispatch' + if: github.event_name == 'schedule' || github.event_name == 'workflow_dispatch'Note: If you need to run only on the Monday noon schedule (not on the Friday 6pm schedule at line 6), consider moving
update-leaderboardto a dedicated.github/workflows/update-leaderboard.ymlfile with just the Monday cron, or add a run condition at the Python script level to check the current time.
398-401: MAJOR: Discord message contains literal backslash-n sequences instead of newlines.Line 398 defines
MESSAGEwith\nescape sequences, but when embedded directly in the JSON string (lines 399–401), the shell treats\nas two literal characters (backslash + 'n'), not as a newline. This causes Discord to display the message with visible\ninstead of line breaks, breaking the formatting.Apply this diff to properly escape the message using
jq:- MESSAGE="🚀 **PR Merged**\n\n**PR #${PR_NUMBER}**: ${TITLE}\n👤 @${AUTHOR}\n🔗 ${PR_URL}\n\nGreat work! Bounty will be processed Friday." + MESSAGE=$(cat <<EOF | jq -Rs . + 🚀 **PR Merged** + + **PR #${PR_NUMBER}**: ${TITLE} + 👤 @${AUTHOR} + 🔗 ${PR_URL} + + Great work! Bounty will be processed Friday. + EOF + ) curl -X POST "$DISCORD_WEBHOOK" \ -H "Content-Type: application/json" \ - -d "{\"content\": \"$MESSAGE\"}" + -d "{\"content\": $MESSAGE}"This uses
jq -Rsto convert the raw multi-line string into a properly escaped JSON string with real newlines, then embeds the JSON string directly (without extra quotes).
302-306: MAJOR: Substring grep for author produces false positives in JSON file.Line 302 uses
grep -q "$AUTHOR" contributors.json, which matches the username anywhere in the file—not just as a key. This can produce false positives: if a username appears in a URL, expertise string, or comment, it will be incorrectly detected as already registered. For example, "alice" appearing in "https://github.com/alice42" would be flagged as a match.Apply this diff to use a JSON-aware check with
jq:if [ ! -f contributors.json ]; then echo "new=true" >> $GITHUB_OUTPUT exit 0 fi - if ! grep -q "$AUTHOR" contributors.json; then + if ! jq -e ".\"$AUTHOR\"" contributors.json >/dev/null 2>&1; then echo "new=true" >> $GITHUB_OUTPUT else echo "new=false" >> $GITHUB_OUTPUT fiThis uses
jq -eto test whether the specific key$AUTHORexists in the JSON object, avoiding false matches from substring searches.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/automation.yml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.8)
.github/workflows/automation.yml
56-56: could not parse as YAML: mapping values are not allowed in this context
(syntax-check)
🪛 YAMLlint (1.37.1)
.github/workflows/automation.yml
[error] 56-56: syntax error: mapping values are not allowed here
(syntax)
🔇 Additional comments (1)
.github/workflows/automation.yml (1)
64-81: Clarify the payment workflow and safeguard duplicate detection.Line 70 checks for duplicate PRs using
payment_history, but the code never updatespayment_historyto mark bounties as paid. Onlypending_bountiesis populated (line 83) and saved (line 84). This means:
- Duplicate detection is broken: The same PR will be re-scanned indefinitely because it's never moved from pending to paid.
- Data structure assumption: The code assumes entries in
payment_historyhave apr_numberfield. If a record is manually added without one, line 70 raisesKeyError.- Incomplete workflow: There's no mechanism to mark a bounty as paid and move it from pending to paid tracking.
Implement a clear payment workflow:
- When a bounty is marked paid (externally or by a future feature), move it from
pending_bountiestopayment_historywith a consistent schema:{pr_number, author, amount, merged_at, payment_method, paid_date}.- Update
save_data()to ensure both files are persisted (the fix at lines 50–54 addresses this).- Safeguard line 70 with
.get('pr_number')to avoidKeyErrorif records lack the field.Consider adding a comment or docstring explaining when/how bounties transition from pending to paid.
|
🎯 Closing for MVP Focus This issue is being closed to help the team focus on MVP-critical features (#1-45). This is NOT abandoned - it's an important feature we'll revisit after MVP completion. Timeline:
Want to work on this anyway? Tracking: Labeled as Thanks for understanding! 🚀 — Mike (@mikejmorgan-ai) |



Automated bounty tracking, leaderboard updates, and Discord notifications
Summary by CodeRabbit