Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughAdds GitHub activity feed integration to the Discord bot, including configuration schema, database migration for polling state tracking, command handler for managing tracked repositories, polling engine with embed builders for multiple event types, and comprehensive test coverage for both command and module functionality. Changes
Possibly related PRs
🚥 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)
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 |
|
Claude finished @BillChirico's task in 3m 22s —— View job Review — PR #110: GitHub Activity Feed
Result: 2 documentation issues remaining (code is clean)The code is solid after 5 fix commits addressing all prior critical issues from previous review rounds (BigInt event ID comparison, 🟡 Warning (2)
Prompt to fix all issues |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Summary — 9 issues found
🔴 Critical (3)
--paginatefetches all pages unnecessarily (src/modules/githubFeed.js:34):--paginatewith-q '.[0:10]'fetches ALL pages (up to 300 events) and applies the jq filter per-page, producing far more than 10 events. Multiple concatenated JSON arrays will also breakJSON.parse(). Remove--paginate.- String comparison for event ID dedup is broken (
src/modules/githubFeed.js:273):e.id > lastEventIduses string comparison on numeric IDs.'9' > '10'istruein JS. UseNumber(e.id) > Number(lastEventId). - Same string comparison bug for newest ID tracking (
src/modules/githubFeed.js:304): Same fix needed.
🟡 Warning (5)
- Direct DB writes bypass
setConfigValue()(src/commands/github.js:150-158):handleAddwrites directly toguild_configtable instead of usingsetConfigValue(), bypassing cache invalidation and change listeners. Also mutates config clone viarepos.push(). - Same direct DB write in
handleRemove(src/commands/github.js:186-193): Same pattern issue. - Same direct DB write in
handleChannel(src/commands/github.js:242-249): Also redundantly callsgetPool(). - Missing
githubinpermissions.allowedCommands(config.json): All other commands have an entry; the newgithubcommand does not. setIntervaltiming is fixed, not dynamic (src/modules/githubFeed.js:375-390): Despite the comment,setIntervalfrequency never changes at runtime. Use self-reschedulingsetTimeoutinstead.- Monkey-patching timer ID with
_firstPoll(src/modules/githubFeed.js:395): Fragile — use a separate module-level variable.
🔵 Nitpick (1)
isValidRepodoesn't validate characters (src/commands/github.js:65-71): Accepts../../etcas valid. Add a regex for GitHub-allowed characters.
📝 Documentation
- AGENTS.md Key Files table should include
src/commands/github.js,src/modules/githubFeed.js, andmigrations/005_github-feed.cjs.
|
| Filename | Overview |
|---|---|
| src/modules/githubFeed.js | New GitHub feed polling module with event filtering, deduplication, and rich embeds. Well-structured with proper error handling, re-entrancy guard, and BigInt comparison for event IDs. |
| src/commands/github.js | GitHub feed management commands with proper admin gating and config persistence via setConfigValue(). Clean validation and error handling throughout. |
| migrations/005_github-feed.cjs | Simple, correct migration adding github_feed_state table with appropriate indexes and constraints for per-guild, per-repo state tracking. |
| src/index.js | Properly wired GitHub feed into startup (after DB ready) and graceful shutdown sequence. Follows existing patterns for background service lifecycle. |
Sequence Diagram
sequenceDiagram
participant Bot as Discord Bot
participant Feed as githubFeed Module
participant GH as GitHub CLI
participant DB as PostgreSQL
participant Discord as Discord Channel
Bot->>Feed: startGithubFeed(client)
Note over Feed: Set 5-minute interval
loop Every 5 minutes
Feed->>Feed: pollAllFeeds()
Note over Feed: Re-entrancy guard active
loop For each guild
Feed->>DB: SELECT last_event_id
DB-->>Feed: Previous event ID
loop For each repo
Feed->>GH: gh api repos/{owner}/{repo}/events
GH-->>Feed: Recent events (up to 10)
Feed->>Feed: Filter new events (BigInt comparison)
alt New events found
loop For each new event
Feed->>Feed: buildEmbed(event, enabledTypes)
alt Event type enabled
Feed->>Discord: safeSend(embed)
Discord-->>Feed: Message sent
end
end
Feed->>DB: UPSERT last_event_id, last_poll_at
else No new events
Feed->>DB: UPDATE last_poll_at
end
end
end
end
Bot->>Feed: stopGithubFeed()
Note over Feed: Clear interval & timeout
Last reviewed commit: 00fdb72
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
Adds a GitHub activity feed feature that polls configured repos and posts Discord embeds for selected GitHub event types, with per-guild configuration and DB-backed dedup state.
Changes:
- Introduces
github_feed_statemigration and a newsrc/modules/githubFeed.jspolling + embed builder module. - Adds
/github feed ...command group for managing tracked repos and target channel. - Wires feed start/stop into bot startup and graceful shutdown; adds config defaults and accompanying tests.
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
src/modules/githubFeed.js |
Implements polling, event fetching via gh, dedup state updates, and embed builders. |
src/commands/github.js |
Adds /github feed subcommands to manage tracked repos/channel. |
src/index.js |
Starts/stops the GitHub feed with bot lifecycle. |
migrations/005_github-feed.cjs |
Adds github_feed_state table for per-guild/per-repo dedup persistence. |
config.json |
Adds github.feed configuration section (disabled by default). |
tests/modules/githubFeed.test.js |
Tests embed builders, fetch logic, feed start/stop, and dedup behavior. |
tests/commands/github.test.js |
Tests /github feed subcommands and admin gating. |
pnpm-lock.yaml |
Locks dependency graph updates (incl. @anthropic-ai/sdk version alignment). |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Summary — 9 issues found
🔴 Critical (4)
handleAddwrites to non-existentguild_configtable (src/commands/github.js:147-161): The config module uses aconfigtable, notguild_config. This INSERT will fail at runtime. Also bypassessetConfigValue()cache/listeners and mutates config clone.handleRemovesameguild_configissue (src/commands/github.js:183-203): Same non-existent table + bypassed config module.handleChannelsameguild_configissue (src/commands/github.js:236-253): Same problem, also redundantly callsgetPool().- Test asserts stale CLI arguments (
tests/modules/githubFeed.test.js:117-122): Commit 81ece78 changedfetchRepoEventsto use?per_page=30and--jq, but the test still asserts--paginateand-q. This test will fail.
🟡 Warning (5)
getPool()throws, never returns null (src/commands/github.js:102-106): Dead code —getPool()throws when pool is null, so theif (!pool)branch is unreachable. Wrap in try/catch.isValidRepodoesn't validate characters (src/commands/github.js:62-68): Accepts../../etcas valid. Add regex for GitHub-allowed characters.setIntervaltiming is fixed, not dynamic (src/modules/githubFeed.js:374-390): Despite the comment, interval frequency never changes. Use self-reschedulingsetTimeout.- Monkey-patching timer with
_firstPoll(src/modules/githubFeed.js:395): Fragile — use a separate module-level variable. - Missing
githubinpermissions.allowedCommands(config.json): All other commands have an entry; the newgithubcommand does not.
📝 Documentation
- AGENTS.md Key Files table should include
src/commands/github.js,src/modules/githubFeed.js, andmigrations/005_github-feed.cjs. - README.md should document the
github.feedconfig section and note theghCLI dependency.
There was a problem hiding this comment.
4 issues found (3 Warning, 2 Nitpick)
🟡 Warning (3)
- Dead
pollIntervalMinutesconfig (config.json:170): Never read bystartGithubFeed(). Remove it or wire it up. - Unhandled
getPool()throw inhandleRemove(src/commands/github.js:158): Throws when DB is uninitialized; wrap in try/catch for a user-friendly error. - No error handling on DB cleanup DELETE (
src/commands/github.js:173-177): Failed DELETE after successfulsetConfigValuegives user a generic error. Wrap in try/catch.
🔵 Nitpick (2)
4. Number() for event ID comparison (src/modules/githubFeed.js:276,307): Will lose precision when IDs exceed MAX_SAFE_INTEGER. Use BigInt.
5. Missing numeric dedup edge case test (tests/modules/githubFeed.test.js:525): Add test with last_event_id: '9' and new event '10'.
📝 Documentation (required per AGENTS.md)
- AGENTS.md Key Files table should include
src/commands/github.js,src/modules/githubFeed.js, andmigrations/005_github-feed.cjs. - README.md should document the
github.feedconfig section and note theghCLI dependency.
- migrations/005_github-feed.cjs: github_feed_state table for per-guild/repo dedup - src/modules/githubFeed.js: polling module with rich embeds (PR/Issue/Release/Push) - src/commands/github.js: /github feed add/remove/list/channel slash command - config.json: github.feed defaults (enabled: false) - src/index.js: wire startGithubFeed / stopGithubFeed on ready/shutdown - tests: 47 new tests covering embeds, dedup, commands, admin gating Closes #51
…prevent over-fetching
…or config writes, fix _firstPoll monkey-patch
- Spread repos array before push to avoid mutating getConfig() clone in-place - Fix misleading comment in startGithubFeed that claimed interval was read from guild config (it's hardcoded to 5 minutes)
BillChirico
force-pushed
the
feat/github-feed
branch
from
303be06 to
adfb4d9
Compare
There was a problem hiding this comment.
Review Summary — 5 issues remaining
Previous review rounds fixed most critical issues (numeric event ID comparison, --paginate removal, setConfigValue usage, _firstPoll separation, isValidRepo regex, github: "everyone" permission, dead pool null-check removal, config clone mutation). Good progress across 4 fix commits.
🟡 Warning (4)
handleRemoveDB cleanup not error-handled (src/commands/github.js:173-178): If DELETE fails aftersetConfigValuesucceeds, user gets a generic error even though the repo was already removed from config. Wrap in try/catch.getPool()throw unhandled inhandleRemove(src/commands/github.js:158):getPool()throws when DB is uninitialized. Wrap in try/catch for a user-friendly error.pollIntervalMinutesdead config (config.json:171): Never read bystartGithubFeed(). Remove it or wire it up to avoid user confusion.- Missing documentation (required per AGENTS.md): Key Files table, Database Tables section, and README.md config reference should include the new GitHub feed files and
github_feed_statetable.
🔵 Nitpick (1)
5. Missing numeric dedup edge case test (tests/modules/githubFeed.test.js:525): Add test with last_event_id: '9' and new event '10' to guard the Number() comparison.
🤖 Prompt to fix all issues
Fix the following issues on branch feat/github-feed:
1. In src/commands/github.js:
a. Line 10: add `warn as logWarn` to the logger import
b. Line 158: wrap `getPool()` in try/catch — on error, call `safeEditReply(interaction, { content: '❌ Database is not available.' })` and return
c. Lines 173-178: wrap the DELETE query in a try/catch. On error, call `logWarn('GitHub feed: failed to clean up state row', { guildId: interaction.guildId, repo, error: err.message })` but still proceed to send the success reply
2. In config.json:
Line 171: remove the `"pollIntervalMinutes": 5` line (and fix trailing comma on previous line)
3. In AGENTS.md:
a. Add to Key Files table:
- `src/commands/github.js` | GitHub feed management slash command
- `src/modules/githubFeed.js` | GitHub activity feed polling engine — interval-based, with embed builders for PR/Issue/Release/Push
- `migrations/005_github-feed.cjs` | Migration for `github_feed_state` table
b. Add to Database Tables section:
- `github_feed_state` | Per-guild, per-repo polling state for GitHub event deduplication
4. In tests/modules/githubFeed.test.js:
After the dedup test at line 524, add a new test:
`it('should correctly dedup with numeric ordering (9 < 10)')` that sets
last_event_id to '9', returns an event with id '10', and asserts safeSend was called
| "repos": [], | ||
| "events": ["pr", "issue", "release", "push"] | ||
| } | ||
| }, |
There was a problem hiding this comment.
🟡 Warning: Missing README.md documentation for github.feed config section
Per AGENTS.md (line 180): "if you add a new config section or key, document it in README.md's config reference". The README has a ⚙️ Configuration section documenting all other config blocks (ai, triage, welcome, moderation, etc.) but github.feed is missing.
Also: the gh CLI is a runtime prerequisite that should be listed in README's Prerequisites section (currently only lists Node.js 22+, Discord bot token, and PostgreSQL).
| @@ -0,0 +1,400 @@ | |||
| /** | |||
There was a problem hiding this comment.
🟡 Warning: Missing AGENTS.md Key Files and Database Tables entries
Per AGENTS.md (lines 183-186):
- "Added a new command → update Key Files table"
- "Added a new module → update Key Files table, document config section"
Add to Key Files table:
| src/commands/github.js | GitHub feed management slash command |
| src/modules/githubFeed.js | GitHub activity feed polling engine with embed builders |
| migrations/005_github-feed.cjs | Migration for github_feed_state table |
Add to Database Tables section:
| github_feed_state | Per-guild, per-repo polling state for GitHub event deduplication |
There was a problem hiding this comment.
2 documentation issues remaining (code is clean). 1) Missing README.md documentation: Per AGENTS.md line 180, the github.feed config section must be documented in README config reference, and gh CLI must be listed in Prerequisites. 2) Missing AGENTS.md updates: Per AGENTS.md lines 183-186, new Key Files entries needed for src/commands/github.js, src/modules/githubFeed.js, migrations/005_github-feed.cjs, and new Database Tables entry for github_feed_state.
Summary
Implements the GitHub activity feed requested in #51.
What's included
Migration
migrations/005_github-feed.cjs:github_feed_statetable to track per-guild, per-repo last seen event ID for dedupModule —
src/modules/githubFeed.jsstartGithubFeed(client)/stopGithubFeed()— interval-based pollinggh api repos/{owner}/{repo}/events(shell exec, 30s timeout)last_event_idin DB (upsert on each poll)safeSendfor all channel messagesCommand —
src/commands/github.js/github feed add owner/repo— admin only/github feed remove owner/repo— admin only/github feed list— everyone/github feed channel #channel— admin onlygithub.feed.enabledConfig
Added to
config.json(disabled by default):Wired into
src/index.jsstartGithubFeed(client)called on startup (after DB ready)stopGithubFeed()called on graceful shutdownTests
47 new tests covering:
fetchRepoEvents(gh CLI mock, empty output, errors)buildEmbeddispatch + enabled event type filteringisValidRepovalidationCloses #51