Conversation
Implements issue #162: reaction role menus. - Add migration 004 creating reaction_role_menus and reaction_role_entries tables - Add src/modules/reactionRoles.js with DB helpers, embed builder, event handlers - Add src/commands/reactionrole.js with /reactionrole create|add|remove|delete|list - Wire handleReactionRoleAdd/Remove into registerReactionHandlers in events.js Roles are granted on reaction add and revoked on reaction remove. All mappings persist in PostgreSQL across bot restarts.
- tests/modules/reactionRoles.test.js: resolveEmojiString, buildReactionRoleEmbed, all DB helpers, handleReactionRoleAdd, handleReactionRoleRemove - tests/commands/reactionrole.test.js: all 5 subcommands (create, add, remove, delete, list) including error paths and guild ownership checks - Fix biome lint: import sort order + unused import removal
|
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 (7)
✨ 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 |
There was a problem hiding this comment.
Pull request overview
Implements “reaction role menus” so guild admins can post a menu message and let members self-assign / self-revoke roles via reaction add/remove events, with mappings persisted in Postgres.
Changes:
- Adds Postgres schema for reaction-role menus + emoji→role entries.
- Introduces
src/modules/reactionRoles.jsfor DB helpers, embed building, and reaction add/remove role handlers. - Adds
/reactionroleslash command to create/manage menus and wires handlers into the global reaction event pipeline.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
migrations/004_reaction_roles.cjs |
Creates reaction_role_menus / reaction_role_entries tables and indexes for persistence. |
src/modules/reactionRoles.js |
DB accessors, embed builder, and add/remove event handlers for granting/revoking roles. |
src/commands/reactionrole.js |
Adds /reactionrole command with create/add/remove/delete/list + embed refresh logic. |
src/modules/events.js |
Calls reaction-role handlers before starboard early-return logic. |
tests/modules/reactionRoles.test.js |
Unit tests for DB helpers, emoji resolution, embed builder, and reaction handlers. |
tests/commands/reactionrole.test.js |
Unit tests for /reactionrole subcommands and error paths. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
| Filename | Overview |
|---|---|
| migrations/004_reaction_roles.cjs | Creates database schema for reaction role menus and entries with proper indexes and CASCADE delete |
| src/modules/reactionRoles.js | Core reaction role logic with DB helpers, event handlers, and embed builder; has getPool() error handling issue where null check won't work since getPool() throws instead of returning null |
| src/commands/reactionrole.js | Slash command with 5 subcommands (create/add/remove/delete/list) for managing reaction role menus; includes role hierarchy validation |
| src/modules/events.js | Integrates reaction role handlers into existing reaction event flow with proper try-catch wrappers before starboard early-return |
Sequence Diagram
sequenceDiagram
participant User
participant Discord
participant Bot
participant DB as PostgreSQL
Note over User,DB: Setup: Admin creates reaction role menu
User->>Bot: /reactionrole create [title]
Bot->>Discord: Post embed with title
Discord-->>Bot: Message created (message_id)
Bot->>DB: INSERT reaction_role_menus
Bot-->>User: Menu created! Use /reactionrole add
User->>Bot: /reactionrole add [message_id] [emoji] [role]
Bot->>DB: SELECT menu by message_id
DB-->>Bot: Menu found
Bot->>DB: UPSERT reaction_role_entry
Bot->>Discord: Add bot reaction to message
Bot->>Discord: Update embed with emoji→role mapping
Bot-->>User: Mapping added
Note over User,DB: Runtime: Member reacts to get role
User->>Discord: Reacts with emoji
Discord->>Bot: messageReactionAdd event
Bot->>DB: SELECT role_id for (message_id, emoji)
DB-->>Bot: role_id found
Bot->>Discord: members.fetch(user_id)
Discord-->>Bot: Member object
Bot->>Discord: member.roles.add(role_id)
Discord-->>User: Role granted
Note over User,DB: Runtime: Member un-reacts to remove role
User->>Discord: Removes reaction
Discord->>Bot: messageReactionRemove event
Bot->>DB: SELECT role_id for (message_id, emoji)
DB-->>Bot: role_id found
Bot->>Discord: member.roles.remove(role_id)
Discord-->>User: Role revoked
Last reviewed commit: d0894b3
* feat: reaction role menus - core module, command, event hooks, migration Implements issue #162: reaction role menus. - Add migration 004 creating reaction_role_menus and reaction_role_entries tables - Add src/modules/reactionRoles.js with DB helpers, embed builder, event handlers - Add src/commands/reactionrole.js with /reactionrole create|add|remove|delete|list - Wire handleReactionRoleAdd/Remove into registerReactionHandlers in events.js Roles are granted on reaction add and revoked on reaction remove. All mappings persist in PostgreSQL across bot restarts. * test: reaction role menus - 40 tests covering module and command - tests/modules/reactionRoles.test.js: resolveEmojiString, buildReactionRoleEmbed, all DB helpers, handleReactionRoleAdd, handleReactionRoleRemove - tests/commands/reactionrole.test.js: all 5 subcommands (create, add, remove, delete, list) including error paths and guild ownership checks - Fix biome lint: import sort order + unused import removal * fix: remove unused import in reactionrole command --------- Co-authored-by: Bill Chirico <[email protected]>
* security: escape user content in triage prompt delimiters (#164) Add escapePromptDelimiters() to HTML-encode < and > in user-supplied message content before it is inserted between XML-style section tags in the LLM prompt. Without escaping, a crafted message containing the literal text `</messages-to-evaluate>` could break out of the user-content section and inject attacker-controlled instructions into the prompt structure. Changes: - Add escapePromptDelimiters(text) utility exported from triage-prompt.js - Apply escape to m.content and m.replyTo.content in buildConversationText() - Add 13 new tests covering the escape function and injection scenarios Closes #164 * security: escape & chars and author fields in prompt delimiters * fix(security): escape & in prompt delimiters and escape author fields - Add & → & escape first in escapePromptDelimiters() to prevent HTML entity bypass attacks (e.g. </messages-to-evaluate>) - Also escape m.author and m.replyTo.author since Discord display names are user-controlled and can contain < / > characters Addresses review feedback on PR #204. * fix: guard replyTo.content before .slice() to handle null/undefined * perf: SQL-based conversation pagination + missing DB indexes (#221) Fixes three performance bottlenecks identified in code review of recently merged features (PR #121 conversations viewer, PR #190 AI feedback). ## Changes ### migrations/004_performance_indexes.cjs (new) Four new indexes targeting hot query paths: - idx_ai_feedback_guild_created (guild_id, created_at DESC) getFeedbackTrend() and getRecentFeedback() filtered by guild_id AND created_at but only had a single-column guild_id index, forcing a full guild scan + sort on every trend/recent call. - idx_conversations_content_trgm (GIN, pg_trgm) content ILIKE '%...%' search was a sequential scan. GIN/trgm index reduces this from O(n) to O(log n * trigram matches). Requires pg_trgm extension (added idempotently). - idx_conversations_guild_created (guild_id, created_at DESC) Default 30-day listing query filters guild_id + created_at. The existing 3-column (guild_id, channel_id, created_at) composite is suboptimal when channel_id is not in the predicate. - idx_flagged_messages_guild_message (guild_id, message_id) Conversation detail + flag endpoints query flagged_messages by guild_id AND message_id = ANY(...). Existing index only covers (guild_id, status). ### src/api/routes/conversations.js **GET / — Replace in-memory pagination with SQL CTE grouping** Before: fetched up to 10,000 message rows into Node memory, grouped them in JavaScript (O(n) time + memory), then sliced for pagination. Every page request loaded the full 10k row dataset. After: single SQL query using window functions (LAG + SUM OVER) to identify conversation boundaries and aggregate summaries directly. COUNT(*) OVER() provides total count without a second query. Pagination happens at the DB with LIMIT/OFFSET on summary rows. Memory overhead is now proportional to page size (default 25), not total conversation volume. Removed now-unused buildConversationSummary() helper (logic inlined into the SQL-side aggregation). **POST /:conversationId/flag — Parallel verification queries** Before: msgCheck and anchorCheck ran sequentially (~2× RTT). After: both run in parallel via Promise.all (1× RTT for verification). ### tests/api/routes/conversations.test.js Updated 'should return paginated conversations' test to mock the new SQL CTE response shape (pre-aggregated summary rows) instead of raw message rows. All 41 conversation tests pass. * feat: channel-level quiet mode via bot mention (#173) (#213) * feat: quiet mode per-channel via bot mention (#173) - Add quietMode.js module with Redis+memory storage - Parse duration from natural language (30m, 1 hour, etc.) - Permission gated via config.quietMode.allowedRoles - Commands: quiet, unquiet, status - Suppress AI responses during quiet mode in events.js - Add quietMode section to config.json (disabled by default) - Add quietMode to configAllowlist.js for dashboard editing * test: add quiet mode tests (41 tests, all passing) * style: fix biome formatting in quietMode.js, events.js, and test * fix(web): fix ai-feedback-stats TypeScript and formatting errors * fix: gate quiet mode checks on enabled flag, validate TTL, honor maxDurationMinutes config - events.js: Wrap isQuietMode() calls in guildConfig.quietMode?.enabled check to avoid unnecessary Redis lookups and prevent stale records from suppressing AI responses when the feature is disabled (PRRT_kwDORICdSM5xdbmp, PRRT_kwDORICdSM5xdbmx) - quietMode.js: Add TTL validation in setQuiet() to guard against 0, negative, or NaN values that would error in Redis (PRRT_kwDORICdSM5xdbm3) - quietMode.js: Update parseDurationFromContent() to accept config parameter and honor guildConfig.quietMode.maxDurationMinutes. Also clamp defaultSeconds to the effective max (PRRT_kwDORICdSM5xdbm_) - configValidation.js: Add quietMode schema entry with enabled, maxDurationMinutes, and allowedRoles properties (PRRT_kwDORICdSM5xdbnH) * style: fix biome formatting in quietMode.js and ai-feedback-stats.tsx * feat: audit log improvements — CSV/JSON export and real-time WebSocket stream (#215) * feat: audit log improvements — CSV/JSON export, real-time WebSocket stream - Add GET /:id/audit-log/export endpoint (CSV and JSON, up to 10k rows) - Add /ws/audit-log WebSocket server for real-time audit entry broadcast - Refactor buildFilters() shared helper to eliminate duplication - Hook broadcastAuditEntry() into insertAuditEntry (RETURNING id+created_at) - Wire setupAuditStream/stopAuditStream into startServer/stopServer lifecycle - Add escapeCsvValue/rowsToCsv helpers with full test coverage - 30 route tests + 17 WebSocket stream tests, all green Closes #136 * fix: PR #215 review feedback - audit stream fixes - ws.ping() crash: guard with readyState check + try/catch to avoid crashing heartbeat interval when socket not OPEN - stopAuditStream race: make setupAuditStream async and await stopAuditStream() to prevent concurrent WebSocketServer creation - Query param array coercion: add typeof === 'string' checks for startDate/endDate to handle Express string|string[]|undefined - CSV CRLF quoting: add \r to RFC 4180 special-char check for proper Windows line ending handling - Test timeouts: make AUTH_TIMEOUT_MS configurable via AUDIT_STREAM_AUTH_TIMEOUT_MS env var, use 100ms in tests * feat: voice channel activity tracking — join/leave/move, leaderboard, export (#212) * feat: add voice_sessions migration (#135) * feat: add voice tracking module — join/leave/move/flush/leaderboard (#135) * feat: wire voiceStateUpdate handler into event registration (#135) * feat: add /voice command — leaderboard, stats, export subcommands (#135) * feat: add voice config defaults to config.json (#135) * feat: wire voice flush start/stop into bot lifecycle (#135) * feat: add voice to config API allowlist (#135) * fix: SQL UPDATE subquery for closeSession, fix import order (#135) * fix(voice): resolve race conditions and missing config schema - Fix openSession: update in-memory state only AFTER DB INSERT succeeds - Fix closeSession: delete from in-memory state only AFTER DB UPDATE succeeds - Fix: allow closeSession on leave/move even when feature is disabled - Fix migration: add UNIQUE constraint to partial index to prevent duplicates - Fix: move 'Voice join' log to after openSession succeeds - Add voice config to CONFIG_SCHEMA for validation --------- Co-authored-by: Bill <[email protected]> * feat(dashboard): auto-save config with 500ms debounce (#199) * feat(dashboard): replace manual save with auto-save (500ms debounce) - Remove 'Save Changes' button; saving now fires automatically 500ms after the last config change (no changes → no network call) - Add saveStatus state ('idle' | 'saving' | 'saved' | 'error') with AutoSaveStatus component showing spinner, check, or error+retry - Add isLoadingConfigRef guard so the initial fetchConfig load never triggers a spurious PATCH - Ctrl+S still works: clears debounce timer and saves immediately - Keep 'beforeunload' warning for validation errors that block save - Replace yellow unsaved-changes banner with a destructive validation error banner (only shown when save is actually blocked) - Error state shows 'Save failed' + 'Retry' button for user recovery Closes #189 * test(dashboard): add auto-save tests for ConfigEditor - No PATCH on initial config load - Validation error banner suppresses auto-save - 'Saving...' spinner visible while PATCH in-flight - 'Save failed' + Retry button on PATCH error * fix(dashboard): prevent fetchConfig from overwriting saveStatus after successful save Add skipSaveStatusReset parameter to fetchConfig so that post-save reloads preserve the 'saved' status indicator instead of immediately resetting to 'idle'. * test(dashboard): use fake timers, restore vi.stubGlobal, fix assertions, add idle/saved coverage - Replace real setTimeout delays with vi.useFakeTimers() + vi.advanceTimersByTimeAsync() for deterministic, fast debounce tests - Add afterEach cleanup: vi.unstubAllGlobals() + vi.useRealTimers() - Replace toBeTruthy() with toBeInTheDocument() for Testing Library queries - Add idle state test (no status indicator shown after load) - Add saved state test (shows 'Saved' after successful save) - Update file-level comment to list all four states --------- Co-authored-by: Bill Chirico <[email protected]> * feat: Reaction role menus (#162) (#205) * feat: reaction role menus - core module, command, event hooks, migration Implements issue #162: reaction role menus. - Add migration 004 creating reaction_role_menus and reaction_role_entries tables - Add src/modules/reactionRoles.js with DB helpers, embed builder, event handlers - Add src/commands/reactionrole.js with /reactionrole create|add|remove|delete|list - Wire handleReactionRoleAdd/Remove into registerReactionHandlers in events.js Roles are granted on reaction add and revoked on reaction remove. All mappings persist in PostgreSQL across bot restarts. * test: reaction role menus - 40 tests covering module and command - tests/modules/reactionRoles.test.js: resolveEmojiString, buildReactionRoleEmbed, all DB helpers, handleReactionRoleAdd, handleReactionRoleRemove - tests/commands/reactionrole.test.js: all 5 subcommands (create, add, remove, delete, list) including error paths and guild ownership checks - Fix biome lint: import sort order + unused import removal * fix: remove unused import in reactionrole command --------- Co-authored-by: Bill Chirico <[email protected]> * fix(security): validate GitHub owner/repo format before gh CLI call (#198) * fix(security): validate GitHub owner/repo format before gh CLI call Prevents API path traversal by validating owner/repo segments against a strict allowlist regex before interpolating them into the gh CLI invocation. Adds: - VALID_GH_NAME regex (/^[a-zA-Z0-9._-]+$/) - isValidGhRepo() helper (exported for testing) - Guard in fetchRepoEvents() — returns [] and warns on invalid input - Strengthened guard in pollGuildFeed() split logic Fixes #160 * test(security): add validation tests for GitHub owner/repo format Covers isValidGhRepo(), VALID_GH_NAME regex, and fetchRepoEvents() validation guard introduced in fix for #160. 19 new tests verify: - Valid alphanumeric/dot/hyphen/underscore names pass - Path traversal (../../etc/passwd) is rejected at both entry points - Slashes, empty strings, non-strings, spaces all rejected - Shell metacharacters (; && $()) blocked - gh CLI is NOT invoked when validation fails - warn() fires with the invalid values (observable audit trail) - Valid owner/repo still reach gh CLI unchanged * fix(security): reject pure-dot owner/repo names to prevent path traversal * test(githubFeed): add tests for pure-dot path traversal bypass --------- Co-authored-by: Bill Chirico <[email protected]> --------- Co-authored-by: Bill <[email protected]> Co-authored-by: Bill Chirico <[email protected]>
| */ | ||
| export async function handleReactionRoleAdd(reaction, user) { | ||
| const pool = getPool(); |
There was a problem hiding this comment.
getPool() throws instead of returning null (see src/db.js:314-318), so the null check on line 183 is unreachable and the try-catch won't catch the error from getPool().
Move getPool() inside the try-catch:
| */ | |
| export async function handleReactionRoleAdd(reaction, user) { | |
| const pool = getPool(); | |
| export async function handleReactionRoleAdd(reaction, user) { | |
| try { | |
| const pool = getPool(); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/modules/reactionRoles.js
Line: 181-183
Comment:
`getPool()` throws instead of returning null (see `src/db.js:314-318`), so the null check on line 183 is unreachable and the try-catch won't catch the error from `getPool()`.
Move `getPool()` inside the try-catch:
```suggestion
export async function handleReactionRoleAdd(reaction, user) {
try {
const pool = getPool();
```
How can I resolve this? If you propose a fix, please make it concise.| const pool = getPool(); | ||
| if (!pool) return; | ||
|
|
There was a problem hiding this comment.
Same issue as handleReactionRoleAdd — getPool() throws instead of returning null, making the null check unreachable.
| const pool = getPool(); | |
| if (!pool) return; | |
| export async function handleReactionRoleRemove(reaction, user) { | |
| try { | |
| const pool = getPool(); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/modules/reactionRoles.js
Line: 233-235
Comment:
Same issue as `handleReactionRoleAdd` — `getPool()` throws instead of returning null, making the null check unreachable.
```suggestion
export async function handleReactionRoleRemove(reaction, user) {
try {
const pool = getPool();
```
How can I resolve this? If you propose a fix, please make it concise.
Summary
Implements #162 — Reaction Role Menus.
Allows server admins to create messages where members can self-assign roles by reacting with specific emojis. Roles are revoked when reactions are removed. All mappings persist in PostgreSQL across bot restarts.
Changes
Migration
migrations/004_reaction_roles.cjs— Createsreaction_role_menusandreaction_role_entriestablesNew module:
src/modules/reactionRoles.jsinsertReactionRoleMenu,findMenuByMessageId,listMenusForGuild,deleteMenu,upsertReactionRoleEntry,removeReactionRoleEntry,getEntriesForMenu,findRoleForReactionbuildReactionRoleEmbed— Discord blurple embed with emoji→role listhandleReactionRoleAdd/handleReactionRoleRemove— grant/revoke role on reaction eventresolveEmojiString— stable string key for both Unicode and custom emojisNew command:
src/commands/reactionrole.js/reactionrole create [title] [channel?] [description?]— Post a new menu/reactionrole add [message_id] [emoji] [role]— Map emoji → role/reactionrole remove [message_id] [emoji]— Remove a mapping/reactionrole delete [message_id]— Delete menu + Discord message/reactionrole list— List all menus in the serverManage Rolespermission; guards against role hierarchy violationsEvents:
src/modules/events.jshandleReactionRoleAdd/Removeinto the existingregisterReactionHandlersbefore the starboard early-return guardTests
tests/modules/reactionRoles.test.jsandtests/commands/reactionrole.test.jsCloses #162