refactor: modularize events.js and add missing tests#240
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughRefactors a monolithic events module into focused submodules under Changes
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
railway-app
bot
temporarily deployed
to
volvox-bot / volvox-bot-pr-240
|
🚅 Deployed to the volvox-bot-pr-240 environment in volvox-bot
|
|
| Filename | Overview |
|---|---|
| src/modules/events.js | Refactored from 959 lines to 72 lines - now serves as clean entry point that imports and re-exports all event handlers from subdirectory modules |
| src/modules/events/messageCreate.js | Extracted message handling logic (279 lines) - includes spam detection, AI triage, moderation, and engagement tracking with proper error handling |
| src/modules/events/interactionCreate.js | Extracted interaction handlers (525 lines) - contains 10 focused handler functions for buttons, modals, and select menus with consistent error handling patterns |
| src/modules/events/reactions.js | Extracted reaction handlers (136 lines) - manages starboard, reaction roles, and AI feedback tracking with proper config gating |
| tests/utils/cronParser.test.js | New comprehensive cron parser tests (15 tests) - covers wildcards, ranges, steps, validation, and edge cases |
| tests/utils/flattenToLeafPaths.test.js | New object flattening tests (11 tests) - validates prototype pollution protection for proto, constructor, and prototype keys |
| tests/api/utils/dangerousKeys.test.js | New dangerous key validation tests (5 tests) - ensures DANGEROUS_KEYS Set contains expected prototype pollution vectors |
| src/utils/flattenToLeafPaths.js | Refactored to import DANGEROUS_KEYS from shared module instead of duplicating the Set definition |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[events.js<br/>Main Entry Point<br/>72 lines] --> B[messageCreate.js<br/>279 lines]
A --> C[interactionCreate.js<br/>525 lines]
A --> D[reactions.js<br/>136 lines]
A --> E[ready.js<br/>52 lines]
A --> F[errors.js<br/>40 lines]
A --> G[guildMemberAdd.js<br/>33 lines]
A --> H[voiceState.js<br/>21 lines]
B --> B1[Spam Detection]
B --> B2[AI Triage]
B --> B3[Moderation]
B --> B4[Engagement Tracking]
C --> C1[Poll Buttons]
C --> C2[Review Claim]
C --> C3[Showcase]
C --> C4[Challenge Buttons]
C --> C5[Ticket System]
C --> C6[Reminders]
C --> C7[Welcome Onboarding]
D --> D1[Starboard]
D --> D2[Reaction Roles]
D --> D3[AI Feedback]
style A fill:#4CAF50
style B fill:#2196F3
style C fill:#2196F3
style D fill:#2196F3
Last reviewed commit: e9096a4
![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
This PR refactors the Discord bot’s event registration into smaller, focused handler modules, expands backend utility test coverage, and significantly refreshes the web landing page UI (including new motion-based components, theming tokens, and fonts). It also adds a GitHub Actions workflow intended to deploy PR previews to Railway.
Changes:
- Split
src/modules/events.jsinto multiple event handler modules undersrc/modules/events/and re-export for compatibility. - Rework the web landing page into modular components (
Hero,FeatureGrid,Pricing,Stats,Footer,InviteButton) and update global theme/fonts; addframer-motion. - Add new Vitest coverage for cron parsing and prototype-pollution-related utilities; add a Railway preview deploy workflow.
Reviewed changes
Copilot reviewed 24 out of 25 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
web/tests/app/landing.test.tsx |
Updates landing page tests; adds Framer Motion mocking and new assertions. |
web/src/components/landing/index.ts |
Adds a barrel export for landing components. |
web/src/components/landing/Stats.tsx |
New stats + testimonials section with animated counters. |
web/src/components/landing/Pricing.tsx |
New pricing section with tier cards and billing toggle. |
web/src/components/landing/InviteButton.tsx |
Extracted invite button component gated by NEXT_PUBLIC_DISCORD_CLIENT_ID. |
web/src/components/landing/Hero.tsx |
New hero section with typewriter effect and preview UI. |
web/src/components/landing/Footer.tsx |
New footer CTA and external links. |
web/src/components/landing/FeatureGrid.tsx |
New feature grid with terminal-style cards and motion. |
web/src/app/page.tsx |
Landing page rewritten to compose new landing components and nav. |
web/src/app/layout.tsx |
Adds font variables (Inter + JetBrains Mono) and updates metadata. |
web/src/app/globals.css |
Introduces “Terminal Chic” theme tokens + animations; tweaks base styling. |
web/package.json |
Adds framer-motion dependency. |
tests/utils/flattenToLeafPaths.test.js |
Adds unit tests for flattenToLeafPaths including dangerous-key skipping. |
tests/utils/cronParser.test.js |
Adds unit tests for cron parsing and next-run calculation. |
tests/api/utils/dangerousKeys.test.js |
Adds tests validating DANGEROUS_KEYS set contents. |
src/modules/events/voiceState.js |
New voice state handler module. |
src/modules/events/ready.js |
New ready handler module with feature-gate logging. |
src/modules/events/reactions.js |
New reactions handler module (starboard, reaction roles, AI feedback). |
src/modules/events/messageCreate.js |
New messageCreate handler module (also currently includes GuildMemberAdd handler). |
src/modules/events/interactionCreate.js |
New interactionCreate handler module (polls, tickets, onboarding, etc.). |
src/modules/events/guildMemberAdd.js |
New guildMemberAdd handler module. |
src/modules/events/errors.js |
New error handler module including process-level error handling. |
src/modules/events.js |
Becomes the entrypoint that imports/re-exports handlers and registers all. |
pnpm-lock.yaml |
Locks framer-motion (and its deps) for the web app. |
.github/workflows/railway-preview.yml |
Adds workflow intended to deploy PR previews to Railway and comment URLs. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Extract handlers from 959-line events.js into focused modules: - ready.js - Client ready handler - messageCreate.js - Message processing (spam, AI, triage) - interactionCreate.js - Slash commands, buttons, modals - reactions.js - Starboard and reaction roles - errors.js - Error handling - voiceState.js - Voice channel tracking - guildMemberAdd.js - Welcome messages Main events.js now imports and re-exports for backward compatibility. Reduced from 959 lines to ~60 lines.
BillChirico
force-pushed
the
feat/code-quality-improvements
branch
from
527aa2c to
ecc6714
Compare
railway-app
bot
temporarily deployed
to
volvox-bot / volvox-bot-pr-240
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/railway-preview.yml:
- Around line 5-6: The workflow's cleanup job never runs because the workflow
trigger (types: [opened, synchronize, reopened]) omits the 'closed' event
checked later, so add 'closed' to the types array so the cleanup conditional can
ever evaluate true; update the trigger list (the types array near the top) to
include "closed" so the existing if condition used by the cleanup job (the check
against github.event.action == 'closed' around the cleanup job) will be
reachable.
In `@src/modules/events.js`:
- Around line 22-25: The import for registerGuildMemberAddHandler is incorrectly
wired from './events/messageCreate.js' instead of the new dedicated module;
update src/modules/events.js so that registerGuildMemberAddHandler (and any
corresponding export) is imported from './events/guildMemberAdd.js' while
leaving registerMessageCreateHandler imported from './events/messageCreate.js',
and make the same correction for the other duplicate import/export occurrences
that reference registerGuildMemberAddHandler (the second occurrence around the
other import block).
In `@src/modules/events/interactionCreate.js`:
- Around line 421-423: The handler currently sends raw err.message to users via
safeEditReply (see safeEditReply usage around interactionCreate error handling);
instead, log the full error server-side (e.g., using your logger or
console.error inside the catch) and change the reply content to a generic,
user-safe message like "❌ An unexpected error occurred. The team has been
notified." Apply the same fix to the other occurrence of safeEditReply at the
second catch (around the repeated block at lines ~458-460) so no raw exception
text is exposed to users.
- Around line 264-267: The conditional check using !interaction.replied prevents
error responses from being sent because handleRulesAcceptButton() and
handleRoleMenuSelection() call deferReply() which sets interaction.replied =
true; update the error paths around safeEditReply to remove the negation and
call safeEditReply(interaction, {...}) directly (or alternatively assert
interaction.replied === true before calling) so the deferred interaction is
edited; apply the same change for the other occurrence referenced (the block
around lines 287-290) and ensure you touch the code paths in the functions:
handleRulesAcceptButton, handleRoleMenuSelection, and the safeEditReply calls
that reference interaction.replied.
In `@src/modules/events/messageCreate.js`:
- Around line 120-121: Replace the empty .catch() on the background call to
trackMessage(...) in the messageCreate handler with proper async error handling:
either convert the surrounding handler to async and await trackMessage(...)
inside a try/catch that logs the caught error, or keep the promise form but
replace .catch(() => {}) with .catch(err => logger.error("trackMessage failed:",
err)) using the project logger (e.g., logger or processLogger); make the same
change for the other occurrence of the background call noted in the comment so
failures are not silently swallowed.
- Around line 21-33: This file defines a duplicate registerGuildMemberAddHandler
that listens for Events.GuildMemberAdd and calls getConfig/sendWelcomeMessage;
remove that function and its related import (sendWelcomeMessage,
recordCommunityActivity if unused) so guildMemberAdd.js is the single owner of
the GuildMemberAdd wiring, leaving messageCreate.js to only handle message
events; ensure no remaining references to registerGuildMemberAddHandler,
Events.GuildMemberAdd, getConfig or sendWelcomeMessage remain in this module
after deletion.
In `@src/modules/events/reactions.js`:
- Around line 41-42: The silent empty-catch around trackReaction (and similar
calls at the other sites) masks async failures; change the callers to use
async/await and wrap await trackReaction(reaction, user) in a try/catch that
logs the error (e.g., processLogger.error or a module logger) and optionally
surfaces or metrics the failure instead of swallowing it. Locate all occurrences
(the immediate call to trackReaction and the other blocks noted around lines
54–61 and 108–112) and replace the .catch(() => {}) pattern with an async
function body using try { await trackReaction(...); } catch (err) {
logger.error('trackReaction failed', { err, reaction, user }); } so failures are
recorded for postmortem.
- Around line 27-35: In the MessageReactionAdd and MessageReactionRemove
handlers, ensure you fetch the reaction itself when reaction.partial is true
before accessing emoji data: check reaction.partial and call await
reaction.fetch() inside a try/catch (similar to the existing
reaction.message.fetch() pattern), returning on failure; then proceed to fetch
reaction.message if reaction.message.partial and use reaction.emoji.name safely.
This applies to the handler functions handling MessageReactionAdd and
MessageReactionRemove so they both fetch reaction first, then message.
In `@src/modules/events/voiceState.js`:
- Around line 16-19: Replace the current pattern of awaiting
handleVoiceStateUpdate(...).catch(...) inside the Events.VoiceStateUpdate
listener with a try/catch block: inside the async callback for
client.on(Events.VoiceStateUpdate, async (oldState, newState) => { ... }), wrap
await handleVoiceStateUpdate(oldState, newState) in try { ... } and catch (err)
{ logError('Voice state update handler error', { error: err.message }); } so
error handling uses try/catch rather than chaining .catch(), keeping the control
flow consistent with async/await conventions.
In `@tests/utils/cronParser.test.js`:
- Around line 64-67: The test uses parseCron('24 * * * *') which is valid (24 is
a valid minute) so update the failing assertion to target an out-of-range hour
value by changing that input to '* 24 * * *' (reference: parseCron in
tests/utils/cronParser.test.js) so it correctly asserts an out-of-range error
for the hour field.
In `@tests/utils/flattenToLeafPaths.test.js`:
- Around line 59-65: The test for flattenToLeafPaths is incorrect because using
a literal __proto__ key doesn't create an own enumerable property; update the
test to create an actual own property named "__proto__" (e.g., use a computed
property like const obj = { safe: 'value', ['__proto__']: 'malicious' } or use
Object.defineProperty to add an own enumerable "__proto__" property) so the
flattenToLeafPaths filtering is exercised; keep the same expectations (result
length 1 and contains ['test.safe','value']) and ensure the malicious
"__proto__" entry is not present.
In `@web/src/app/globals.css`:
- Around line 159-161: Add a blank line between the two declarations inside the
body block to satisfy the Stylelint declaration-empty-line-before rule:
specifically, in the body selector place an empty line between the `@apply`
declaration (`@apply` bg-background text-foreground antialiased;) and the
font-feature-settings declaration (font-feature-settings: "rlig" 1, "calt" 1;).
In `@web/src/app/page.tsx`:
- Around line 20-52: The header nav packs many controls into a single row and
will overflow on small screens; update the nav rendering (the <nav> containing
ThemeToggle, Button/Link Sign In, and InviteButton) to be responsive by adding a
mobile fallback: convert the inline nav into a responsive container that stacks
or collapses on small screens (e.g., use a hidden/visible mobile menu or
flex-wrap/column at small breakpoints), add a mobile menu toggle (hamburger)
that shows the links when opened, and ensure ThemeToggle, Sign In (Button +
Link), and InviteButton are reachable via that menu; adjust Tailwind classes on
the nav and the specific components (ThemeToggle, Button/Link, InviteButton) so
desktop remains same while small screens use stacked layout or collapsed menu.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3a3e2f18-51c4-4078-8d8d-ec968740efe2
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (24)
.github/workflows/railway-preview.ymlsrc/modules/events.jssrc/modules/events/errors.jssrc/modules/events/guildMemberAdd.jssrc/modules/events/interactionCreate.jssrc/modules/events/messageCreate.jssrc/modules/events/reactions.jssrc/modules/events/ready.jssrc/modules/events/voiceState.jstests/api/utils/dangerousKeys.test.jstests/utils/cronParser.test.jstests/utils/flattenToLeafPaths.test.jsweb/package.jsonweb/src/app/globals.cssweb/src/app/layout.tsxweb/src/app/page.tsxweb/src/components/landing/FeatureGrid.tsxweb/src/components/landing/Footer.tsxweb/src/components/landing/Hero.tsxweb/src/components/landing/InviteButton.tsxweb/src/components/landing/Pricing.tsxweb/src/components/landing/Stats.tsxweb/src/components/landing/index.tsweb/tests/app/landing.test.tsx
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Agent
- GitHub Check: Greptile Review
🧰 Additional context used
📓 Path-based instructions (6)
{src,web}/**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Winston logger from
src/logger.js, NEVER useconsole.*
Files:
web/src/components/landing/FeatureGrid.tsxsrc/modules/events/voiceState.jsweb/src/components/landing/Stats.tsxsrc/modules/events/messageCreate.jssrc/modules/events/ready.jsweb/src/components/landing/Footer.tsxweb/src/components/landing/InviteButton.tsxweb/tests/app/landing.test.tsxsrc/modules/events/reactions.jsweb/src/components/landing/Hero.tsxsrc/modules/events/errors.jsweb/src/components/landing/Pricing.tsxsrc/modules/events/interactionCreate.jssrc/modules/events/guildMemberAdd.jsweb/src/components/landing/index.tssrc/modules/events.jsweb/src/app/page.tsxweb/src/app/layout.tsx
web/**/*.{ts,tsx,jsx,js}
📄 CodeRabbit inference engine (AGENTS.md)
Use Next.js 16 App Router for web dashboard, with Discord OAuth2 authentication, dark/light theme support, and mobile-responsive design
Files:
web/src/components/landing/FeatureGrid.tsxweb/src/components/landing/Stats.tsxweb/src/components/landing/Footer.tsxweb/src/components/landing/InviteButton.tsxweb/tests/app/landing.test.tsxweb/src/components/landing/Hero.tsxweb/src/components/landing/Pricing.tsxweb/src/components/landing/index.tsweb/src/app/page.tsxweb/src/app/layout.tsx
src/**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{js,ts,jsx,tsx}: Use ESM only — Useimport/export, no CommonJS
Use single quotes for strings — No double quotes except in JSON
Always require semicolons at end of statements
Use 2-space indent, enforced by Biome
Always use async/await for asynchronous operations and promise handling
Files:
src/modules/events/voiceState.jssrc/modules/events/messageCreate.jssrc/modules/events/ready.jssrc/modules/events/reactions.jssrc/modules/events/errors.jssrc/modules/events/interactionCreate.jssrc/modules/events/guildMemberAdd.jssrc/modules/events.js
src/**/*.{js,ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use parameterized SQL queries — Never use string interpolation in database queries
Files:
src/modules/events/voiceState.jssrc/modules/events/messageCreate.jssrc/modules/events/ready.jssrc/modules/events/reactions.jssrc/modules/events/errors.jssrc/modules/events/interactionCreate.jssrc/modules/events/guildMemberAdd.jssrc/modules/events.js
src/modules/**/*.{js,ts}
📄 CodeRabbit inference engine (AGENTS.md)
src/modules/**/*.{js,ts}: Config section additions MUST be added toSAFE_CONFIG_KEYSinsrc/api/utils/configAllowlist.jsto enable API saves
Gate all community features behindconfig.<feature>.enabledconfiguration checks
Redis caching should usesrc/utils/cache.jsfor generic caching with Redis + in-memory fallback
Use Discord cache utilities —src/utils/discordCache.jsfor channels/roles/members,src/utils/reputationCache.jsfor leaderboard/rank data
Files:
src/modules/events/voiceState.jssrc/modules/events/messageCreate.jssrc/modules/events/ready.jssrc/modules/events/reactions.jssrc/modules/events/errors.jssrc/modules/events/interactionCreate.jssrc/modules/events/guildMemberAdd.jssrc/modules/events.js
tests/**/*.{js,ts}
📄 CodeRabbit inference engine (AGENTS.md)
Maintain 80% test coverage threshold — Never lower the coverage requirement
Files:
tests/utils/cronParser.test.jstests/utils/flattenToLeafPaths.test.jstests/api/utils/dangerousKeys.test.js
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-02T21:23:59.512Z
Learning: Applies to web/**/*.{ts,tsx,jsx,js} : Use Next.js 16 App Router for web dashboard, with Discord OAuth2 authentication, dark/light theme support, and mobile-responsive design
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-02T21:23:59.512Z
Learning: Create modules in `src/modules/` for new features, add config section to `config.json`, update `SAFE_CONFIG_KEYS`, create slash command if needed, add database migration if needed, write tests, and update dashboard UI if configurable
📚 Learning: 2026-03-02T21:23:59.512Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-02T21:23:59.512Z
Learning: Applies to web/**/*.{ts,tsx,jsx,js} : Use Next.js 16 App Router for web dashboard, with Discord OAuth2 authentication, dark/light theme support, and mobile-responsive design
Applied to files:
web/package.jsonweb/tests/app/landing.test.tsxweb/src/app/page.tsxweb/src/app/layout.tsx
📚 Learning: 2026-03-02T21:23:59.512Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-02T21:23:59.512Z
Learning: Applies to tests/**/*.{js,ts} : Maintain 80% test coverage threshold — Never lower the coverage requirement
Applied to files:
tests/utils/cronParser.test.js
📚 Learning: 2026-03-02T21:23:59.512Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-02T21:23:59.512Z
Learning: Applies to src/modules/**/*.{js,ts} : Config section additions MUST be added to `SAFE_CONFIG_KEYS` in `src/api/utils/configAllowlist.js` to enable API saves
Applied to files:
tests/api/utils/dangerousKeys.test.js
🪛 Stylelint (17.3.0)
web/src/app/globals.css
[error] 4-4: Unexpected unknown at-rule "@theme" (scss/at-rule-no-unknown)
(scss/at-rule-no-unknown)
[error] 160-160: Expected empty line before declaration (declaration-empty-line-before)
(declaration-empty-line-before)
🔇 Additional comments (18)
tests/api/utils/dangerousKeys.test.js (1)
1-27: Good focused contract coverage forDANGEROUS_KEYS.These tests clearly validate membership, negatives, and type/size of the exported set.
tests/utils/flattenToLeafPaths.test.js (1)
5-57: Strong behavior coverage for flattening, arrays, and edge cases.The suite is comprehensive and readable across normal and nested shapes.
Also applies to: 67-109
tests/utils/cronParser.test.js (2)
4-116: Good coverage depth across parser and scheduler behaviorsNice breadth here: wildcards, single values, list/range/step parsing, validation failures, and next-run scenarios (including no-match horizon) all improve confidence in
cronParser.As per coding guidelines, "Maintain 80% test coverage threshold — Never lower the coverage requirement".
83-107: Use local time construction to avoid timezone-dependent test failuresThese tests construct UTC timestamps (
...Z) but thegetNextCronRun()function operates entirely in local time (it usesgetHours(),getDate(), etc. internally). The mismatch causes timezone-dependent failures. Replace UTC timestamp construction with local time construction:new Date(2024, 5, 15, 10, 0, 0)(note: month is 0-indexed), or use timestamp comparison (.getTime()) which is timezone-independent.Suggested fix
- const from = new Date('2024-06-15T10:00:00Z'); + const from = new Date(2024, 5, 15, 10, 0, 0); const next = getNextCronRun(cron, from); expect(next.getHours()).toBe(12); expect(next.getMinutes()).toBe(0); expect(next.getDate()).toBe(15); @@ - const from = new Date('2024-06-15T14:00:00Z'); + const from = new Date(2024, 5, 15, 14, 0, 0); const next = getNextCronRun(cron, from); expect(next.getDate()).toBe(16); expect(next.getHours()).toBe(12); @@ - const from = new Date('2024-06-15T10:00:00Z'); + const from = new Date(2024, 5, 15, 10, 0, 0); const next = getNextCronRun(cron, from); expect(next.getMinutes()).toBe(30); expect(next.getHours()).toBe(10);> Likely an incorrect or invalid review comment.web/package.json (1)
25-25: No blocking concern in this manifest edit.web/src/app/layout.tsx (1)
24-30: Root layout update looks internally consistent.web/src/components/landing/InviteButton.tsx (1)
13-23: Conditional CTA rendering is implemented cleanly.web/src/components/landing/Stats.tsx (1)
13-33: Animation lifecycle handling is correct in this effect block.web/src/components/landing/Pricing.tsx (1)
155-176: CTA fallback and disabled-state handling are implemented correctly.web/src/components/landing/Footer.tsx (1)
29-47: Invite CTA branching is clear and behaves as expected.web/src/app/globals.css (1)
4-4: No linting configuration issue exists.The
.stylelintrc.jsonfile is unused (not referenced in code, no stylelint dependency, not part of the lint script). The project uses Biome as its linter, which is configured only for TypeScript/TSX files inweb/src/**/*.{ts,tsx}and explicitly excludes CSS files. The@themeat-rule inglobals.cssis not checked by any linter and requires no configuration changes.> Likely an incorrect or invalid review comment.web/src/components/landing/FeatureGrid.tsx (1)
36-109: Clean, reusable feature-card composition with motion fallbacks.
TerminalCard+features.map(...)is well-structured, and reduced-motion handling is correctly integrated for entry/hover behavior.web/src/components/landing/index.ts (1)
1-6: Barrel export is straightforward and improves import ergonomics.This keeps landing-component consumption clean without adding coupling.
web/src/components/landing/Hero.tsx (1)
11-46: Typewriter lifecycle and Hero composition look solid.The timeout/interval cleanup is correct, and the section composition is clear and maintainable.
Also applies to: 153-213
web/tests/app/landing.test.tsx (1)
7-24: Good test hardening for animated UI + env-dependent CTA behavior.The framer-motion mock and env reset pattern make these landing tests deterministic and less flaky.
Also applies to: 29-87
src/modules/events/ready.js (1)
20-51: Ready handler modularization looks solid.Good separation of startup concerns and clean feature-gate logging.
src/modules/events/errors.js (1)
21-39: Guarded process-level handler registration is well done.The one-time registration guard prevents duplicate listeners and keeps startup idempotent.
src/modules/events/guildMemberAdd.js (1)
15-19: Dedicated guild-member-add handler module looks good.The handler is small, focused, and uses per-guild config resolution cleanly.
railway-app
bot
temporarily deployed
to
volvox-bot / volvox-bot-pr-240
railway-app
bot
temporarily deployed
to
volvox-bot / volvox-bot-pr-240
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (8)
tests/utils/cronParser.test.js (1)
64-67:⚠️ Potential issue | 🟡 MinorFix the invalid out-of-range assertion input.
Line 66 uses
parseCron('24 * * * *'), but24is valid for the minute field (0-59), so this does not test an out-of-range case.Suggested fix
it('should reject out-of-range values', () => { expect(() => parseCron('60 * * * *')).toThrow('Invalid cron value'); - expect(() => parseCron('24 * * * *')).toThrow('Invalid cron value'); + expect(() => parseCron('* 24 * * *')).toThrow('Invalid cron value'); expect(() => parseCron('* 25 * * *')).toThrow('Invalid cron value'); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/utils/cronParser.test.js` around lines 64 - 67, The test "should reject out-of-range values" is asserting an out-of-range minute but uses `parseCron('24 * * * *')` which is actually valid for the minute field; change that input to place 24 in the hour position (e.g., `'* 24 * * *'`) so the assertion tests an invalid hour value, updating the expectation that `parseCron` throws for that case; refer to the `parseCron` function and the test case name "should reject out-of-range values" to locate and modify the line.src/modules/events.js (1)
22-25:⚠️ Potential issue | 🟠 MajorWire
registerGuildMemberAddHandlerfrom./events/guildMemberAdd.js.Current import source keeps guild-member-add coupled to
messageCreate.js, which undermines this refactor’s modular boundary.Proposed fix
import { - registerGuildMemberAddHandler, registerMessageCreateHandler, } from './events/messageCreate.js'; +import { registerGuildMemberAddHandler } from './events/guildMemberAdd.js';Also applies to: 33-35, 59-60
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/events.js` around lines 22 - 25, The import for registerGuildMemberAddHandler is incorrectly pulled from './events/messageCreate.js'; update all imports that bring in registerGuildMemberAddHandler (currently referencing messageCreate.js) to instead import from './events/guildMemberAdd.js' so the guild-member-add handler is wired from its own module (search for registerGuildMemberAddHandler in this file and replace the source module accordingly).src/modules/events/voiceState.js (1)
16-19: 🛠️ Refactor suggestion | 🟠 MajorUse
try/catchdirectly in the async listener.Line 17 mixes
awaitwith chained.catch(). Keep async flow consistent with atry/catchblock aroundawait handleVoiceStateUpdate(...).Proposed fix
export function registerVoiceStateHandler(client) { client.on(Events.VoiceStateUpdate, async (oldState, newState) => { - await handleVoiceStateUpdate(oldState, newState).catch((err) => { - logError('Voice state update handler error', { error: err.message }); - }); + try { + await handleVoiceStateUpdate(oldState, newState); + } catch (err) { + logError('Voice state update handler error', { error: err?.message }); + } }); }As per coding guidelines "Always use async/await for asynchronous operations and promise handling."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/events/voiceState.js` around lines 16 - 19, The listener for Events.VoiceStateUpdate mixes await with a chained .catch; replace that pattern by wrapping the await handleVoiceStateUpdate(oldState, newState) call inside a try/catch inside the async callback (the client.on handler) and move the logError('Voice state update handler error', { error: err.message }) into the catch block so errors are handled via async/await flow consistently.src/modules/events/messageCreate.js (2)
21-33:⚠️ Potential issue | 🟠 MajorRemove
registerGuildMemberAddHandlerfrom this module to avoid split ownership.This handler should have a single owner (
src/modules/events/guildMemberAdd.js) to prevent wiring drift and duplicate maintenance.Proposed fix
-import { recordCommunityActivity, sendWelcomeMessage } from '../welcome.js'; +import { recordCommunityActivity } from '../welcome.js'; @@ -export function registerGuildMemberAddHandler(client, _config) { - client.on(Events.GuildMemberAdd, async (member) => { - const guildConfig = getConfig(member.guild.id); - await sendWelcomeMessage(member, client, guildConfig); - }); -}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/events/messageCreate.js` around lines 21 - 33, Remove the exported registerGuildMemberAddHandler function and its GuildMemberAdd listener wiring from this module to eliminate split ownership; delete the function definition that registers client.on(Events.GuildMemberAdd, ...) as well as any now-unused imports that only served that handler (e.g., sendWelcomeMessage, getConfig or Events if they become unused), leaving guildMemberAdd.js as the single owner of the GuildMemberAdd handling logic.
120-121: 🛠️ Refactor suggestion | 🟠 MajorReplace empty async catches with logged
try/catchhandling.Line 120 and Line 205 currently swallow failures. At minimum, log these failures for operational visibility.
As per coding guidelines "Always use async/await for asynchronous operations and promise handling."
Also applies to: 205-206
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/events/messageCreate.js` around lines 120 - 121, Replace the empty promise swallow for trackMessage(message).catch(() => {}) with proper async/await and try/catch: call await trackMessage(message) inside an async function or the surrounding handler, catch the error and log it (using the project's logger, e.g. processLogger.error or console.error) including contextual info (message id/user); apply the same change to the other empty .catch occurrences in this file so no async errors are silently ignored.src/modules/events/reactions.js (2)
27-35:⚠️ Potential issue | 🟠 MajorFetch partial reactions before reading emoji data.
Line 45 and Line 103 read
reaction.emoji.namewithout first resolvingreaction.partial. Add areaction.fetch()guard in both add/remove handlers.Proposed fix
client.on(Events.MessageReactionAdd, async (reaction, user) => { // Ignore bot reactions if (user.bot) return; + if (reaction.partial) { + try { + await reaction.fetch(); + } catch { + return; + } + } @@ client.on(Events.MessageReactionRemove, async (reaction, user) => { if (user.bot) return; + if (reaction.partial) { + try { + await reaction.fetch(); + } catch { + return; + } + }Also applies to: 45-45, 89-95, 103-103
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/events/reactions.js` around lines 27 - 35, The handlers that read reaction.emoji.name must ensure the Reaction itself is resolved first; add the same partial fetch guard used for reaction.message to the reaction add/remove handlers: if (reaction.partial) { try { await reaction.fetch(); } catch { return; } } before any access to reaction.emoji or reaction.emoji.name (e.g., in your messageReactionAdd/messageReactionRemove or the add/remove handler functions), so the code safely resolves partial Reaction objects and returns early on fetch failure.
40-42: 🛠️ Refactor suggestion | 🟠 MajorDo not silently swallow async failures in reaction side-effects.
These branches drop operational failures with no signal. Replace empty catches with explicit
try/catch+ structured logging.As per coding guidelines "Always use async/await for asynchronous operations and promise handling."
Also applies to: 54-61, 108-112
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/events/reactions.js` around lines 40 - 42, The current fire-and-forget calls to trackReaction(reaction, user) (and the similar usages at the other noted ranges) silently swallow errors via .catch(() => {}); change these to use async/await with a try/catch (or attach a .catch that calls the existing logger) so failures are logged and not dropped: locate each call to trackReaction in src/modules/events/reactions.js and wrap the invocation in a try { await trackReaction(reaction, user); } catch (err) { logger.error("trackReaction failed", { reaction, userId: user?.id, error: err }); } (or the project’s structured logging helper) so errors are captured with context rather than ignored.src/modules/events/interactionCreate.js (1)
421-423:⚠️ Potential issue | 🟠 MajorDo not expose raw exception messages to users in ticket flows.
Line 422 and Line 459 echo
err.messagedirectly. Log details server-side and return a generic user-safe failure message.Proposed fix
} catch (err) { + logError('Ticket open failed', { + userId: interaction.user?.id, + guildId: interaction.guildId, + error: err?.message, + }); await safeEditReply(interaction, { - content: `❌ ${err.message}`, + content: '❌ Something went wrong while opening your ticket. Please try again.', }); } @@ } catch (err) { + logError('Ticket close failed', { + userId: interaction.user?.id, + guildId: interaction.guildId, + error: err?.message, + }); await safeEditReply(interaction, { - content: `❌ ${err.message}`, + content: '❌ Something went wrong while closing this ticket. Please try again.', }); }Also applies to: 458-460
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/events/interactionCreate.js` around lines 421 - 423, Replace instances where err.message is sent back to users (e.g., the safeEditReply calls in the interactionCreate handler) with a generic, user-safe failure message like "An unexpected error occurred while processing your ticket. Please try again or contact support." Instead, log the full error server-side using the existing logger or console.error with context (include interaction id, user id, and stack) so developers can debug. Update both occurrences that echo err.message (the two safeEditReply calls in the ticket flow) and ensure no other direct err.message exposures remain in the interactionCreate logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/modules/events/guildMemberAdd.js`:
- Around line 16-19: The GuildMemberAdd handler currently calls getConfig and
sendWelcomeMessage unconditionally; update the Events.GuildMemberAdd callback to
first retrieve guildConfig via getConfig(member.guild.id), check that
guildConfig?.welcome?.enabled is true before calling sendWelcomeMessage, and if
the gate fails log a debug/info message (using the same logger pattern as
elsewhere) indicating welcome messages are disabled for that guild; ensure any
errors from getConfig or sendWelcomeMessage are caught and logged so failures
are visible.
In `@tests/api/utils/dangerousKeys.test.js`:
- Line 2: The import in dangerousKeys.test.js uses an incorrect relative path to
DANGEROUS_KEYS; update the import statement that references DANGEROUS_KEYS so it
climbs one directory higher (e.g., change the path from
'../../src/api/utils/dangerousKeys.js' to
'../../../src/api/utils/dangerousKeys.js') so the test file correctly imports
the module.
In `@tests/utils/flattenToLeafPaths.test.js`:
- Around line 101-107: The test exposes that flattenToLeafPaths builds paths
with a leading dot when given an empty prefix due to the concat line (const path
= `${prefix}.${key}`); change flattenToLeafPaths to treat an empty prefix as the
root by computing path with a conditional (e.g., if prefix is falsy use key else
`${prefix}.${key}`) so root keys become 'key' not '.key', and update the test
case in flattenToLeafPaths.test.js to expect ['key','value'] instead of
['.key','value'].
---
Duplicate comments:
In `@src/modules/events.js`:
- Around line 22-25: The import for registerGuildMemberAddHandler is incorrectly
pulled from './events/messageCreate.js'; update all imports that bring in
registerGuildMemberAddHandler (currently referencing messageCreate.js) to
instead import from './events/guildMemberAdd.js' so the guild-member-add handler
is wired from its own module (search for registerGuildMemberAddHandler in this
file and replace the source module accordingly).
In `@src/modules/events/interactionCreate.js`:
- Around line 421-423: Replace instances where err.message is sent back to users
(e.g., the safeEditReply calls in the interactionCreate handler) with a generic,
user-safe failure message like "An unexpected error occurred while processing
your ticket. Please try again or contact support." Instead, log the full error
server-side using the existing logger or console.error with context (include
interaction id, user id, and stack) so developers can debug. Update both
occurrences that echo err.message (the two safeEditReply calls in the ticket
flow) and ensure no other direct err.message exposures remain in the
interactionCreate logic.
In `@src/modules/events/messageCreate.js`:
- Around line 21-33: Remove the exported registerGuildMemberAddHandler function
and its GuildMemberAdd listener wiring from this module to eliminate split
ownership; delete the function definition that registers
client.on(Events.GuildMemberAdd, ...) as well as any now-unused imports that
only served that handler (e.g., sendWelcomeMessage, getConfig or Events if they
become unused), leaving guildMemberAdd.js as the single owner of the
GuildMemberAdd handling logic.
- Around line 120-121: Replace the empty promise swallow for
trackMessage(message).catch(() => {}) with proper async/await and try/catch:
call await trackMessage(message) inside an async function or the surrounding
handler, catch the error and log it (using the project's logger, e.g.
processLogger.error or console.error) including contextual info (message
id/user); apply the same change to the other empty .catch occurrences in this
file so no async errors are silently ignored.
In `@src/modules/events/reactions.js`:
- Around line 27-35: The handlers that read reaction.emoji.name must ensure the
Reaction itself is resolved first; add the same partial fetch guard used for
reaction.message to the reaction add/remove handlers: if (reaction.partial) {
try { await reaction.fetch(); } catch { return; } } before any access to
reaction.emoji or reaction.emoji.name (e.g., in your
messageReactionAdd/messageReactionRemove or the add/remove handler functions),
so the code safely resolves partial Reaction objects and returns early on fetch
failure.
- Around line 40-42: The current fire-and-forget calls to
trackReaction(reaction, user) (and the similar usages at the other noted ranges)
silently swallow errors via .catch(() => {}); change these to use async/await
with a try/catch (or attach a .catch that calls the existing logger) so failures
are logged and not dropped: locate each call to trackReaction in
src/modules/events/reactions.js and wrap the invocation in a try { await
trackReaction(reaction, user); } catch (err) { logger.error("trackReaction
failed", { reaction, userId: user?.id, error: err }); } (or the project’s
structured logging helper) so errors are captured with context rather than
ignored.
In `@src/modules/events/voiceState.js`:
- Around line 16-19: The listener for Events.VoiceStateUpdate mixes await with a
chained .catch; replace that pattern by wrapping the await
handleVoiceStateUpdate(oldState, newState) call inside a try/catch inside the
async callback (the client.on handler) and move the logError('Voice state update
handler error', { error: err.message }) into the catch block so errors are
handled via async/await flow consistently.
In `@tests/utils/cronParser.test.js`:
- Around line 64-67: The test "should reject out-of-range values" is asserting
an out-of-range minute but uses `parseCron('24 * * * *')` which is actually
valid for the minute field; change that input to place 24 in the hour position
(e.g., `'* 24 * * *'`) so the assertion tests an invalid hour value, updating
the expectation that `parseCron` throws for that case; refer to the `parseCron`
function and the test case name "should reject out-of-range values" to locate
and modify the line.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: ce359cc4-ba4b-49d0-bd67-5ac8a3154744
📒 Files selected for processing (11)
src/modules/events.jssrc/modules/events/errors.jssrc/modules/events/guildMemberAdd.jssrc/modules/events/interactionCreate.jssrc/modules/events/messageCreate.jssrc/modules/events/reactions.jssrc/modules/events/ready.jssrc/modules/events/voiceState.jstests/api/utils/dangerousKeys.test.jstests/utils/cronParser.test.jstests/utils/flattenToLeafPaths.test.js
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Greptile Review
🧰 Additional context used
📓 Path-based instructions (5)
src/**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{js,ts,jsx,tsx}: Use ESM only — Useimport/export, no CommonJS
Use single quotes for strings — No double quotes except in JSON
Always require semicolons at end of statements
Use 2-space indent, enforced by Biome
Always use async/await for asynchronous operations and promise handling
Files:
src/modules/events/ready.jssrc/modules/events/voiceState.jssrc/modules/events/errors.jssrc/modules/events/guildMemberAdd.jssrc/modules/events/interactionCreate.jssrc/modules/events/reactions.jssrc/modules/events.jssrc/modules/events/messageCreate.js
{src,web}/**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Winston logger from
src/logger.js, NEVER useconsole.*
Files:
src/modules/events/ready.jssrc/modules/events/voiceState.jssrc/modules/events/errors.jssrc/modules/events/guildMemberAdd.jssrc/modules/events/interactionCreate.jssrc/modules/events/reactions.jssrc/modules/events.jssrc/modules/events/messageCreate.js
src/**/*.{js,ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use parameterized SQL queries — Never use string interpolation in database queries
Files:
src/modules/events/ready.jssrc/modules/events/voiceState.jssrc/modules/events/errors.jssrc/modules/events/guildMemberAdd.jssrc/modules/events/interactionCreate.jssrc/modules/events/reactions.jssrc/modules/events.jssrc/modules/events/messageCreate.js
src/modules/**/*.{js,ts}
📄 CodeRabbit inference engine (AGENTS.md)
src/modules/**/*.{js,ts}: Config section additions MUST be added toSAFE_CONFIG_KEYSinsrc/api/utils/configAllowlist.jsto enable API saves
Gate all community features behindconfig.<feature>.enabledconfiguration checks
Redis caching should usesrc/utils/cache.jsfor generic caching with Redis + in-memory fallback
Use Discord cache utilities —src/utils/discordCache.jsfor channels/roles/members,src/utils/reputationCache.jsfor leaderboard/rank data
Files:
src/modules/events/ready.jssrc/modules/events/voiceState.jssrc/modules/events/errors.jssrc/modules/events/guildMemberAdd.jssrc/modules/events/interactionCreate.jssrc/modules/events/reactions.jssrc/modules/events.jssrc/modules/events/messageCreate.js
tests/**/*.{js,ts}
📄 CodeRabbit inference engine (AGENTS.md)
Maintain 80% test coverage threshold — Never lower the coverage requirement
Files:
tests/utils/cronParser.test.jstests/api/utils/dangerousKeys.test.jstests/utils/flattenToLeafPaths.test.js
🧠 Learnings (3)
📚 Learning: 2026-03-02T21:23:59.512Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-02T21:23:59.512Z
Learning: Applies to src/**/*.{js,ts,jsx,tsx} : Always use async/await for asynchronous operations and promise handling
Applied to files:
src/modules/events/voiceState.jssrc/modules/events/reactions.jssrc/modules/events/messageCreate.js
📚 Learning: 2026-03-02T21:23:59.512Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-02T21:23:59.512Z
Learning: Applies to src/{commands,index}.{js,ts} : Use safe Discord message methods — Use `safeReply()`, `safeSend()`, or `safeEditReply()` instead of direct Discord.js methods
Applied to files:
src/modules/events/interactionCreate.js
📚 Learning: 2026-03-02T21:23:59.512Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-02T21:23:59.512Z
Learning: Applies to src/modules/**/*.{js,ts} : Config section additions MUST be added to `SAFE_CONFIG_KEYS` in `src/api/utils/configAllowlist.js` to enable API saves
Applied to files:
tests/api/utils/dangerousKeys.test.js
🪛 GitHub Actions: CI
tests/api/utils/dangerousKeys.test.js
[error] 2-2: Cannot find module '../../src/api/utils/dangerousKeys.js' imported by tests/api/utils/dangerousKeys.test.js
🔇 Additional comments (9)
tests/api/utils/dangerousKeys.test.js (1)
4-27: Test logic is well-structured.The tests comprehensively verify both positive cases (dangerous keys present) and negative cases (safe keys absent), along with type and size validation. Once the import path is fixed, these tests will provide good coverage for the
DANGEROUS_KEYSexport.tests/utils/flattenToLeafPaths.test.js (5)
1-14: Import path and basic flattening tests look correct.The import path correctly resolves to
src/utils/flattenToLeafPaths.js. The basic flattening tests appropriately verify primitive value handling and path string construction.
16-56: Nested object and array handling tests are thorough.The tests correctly verify dot-notation path construction for nested objects and the important behavior that arrays are treated as leaf values without recursion into elements.
59-65: Test does not actually exercise__proto__filtering.As previously noted, using
{ __proto__: 'malicious' }with a non-computed key sets the object's[[Prototype]]rather than creating an own enumerable property. The test passes regardless of whether filtering logic exists.🔧 Use computed property syntax to create actual own property
it('should skip __proto__', () => { - const obj = { safe: 'value', __proto__: 'malicious' }; + const obj = { safe: 'value', ['__proto__']: 'malicious' }; + expect(Object.prototype.hasOwnProperty.call(obj, '__proto__')).toBe(true); const result = flattenToLeafPaths(obj, 'test'); expect(result).toHaveLength(1); expect(result).toContainEqual(['test.safe', 'value']); });
67-81:constructorandprototypetests are valid.Unlike
__proto__, these keys create normal own enumerable properties when used in object literals, so these tests correctly exercise the dangerous-key filtering logic.
84-99: Empty object and null value edge cases are well covered.These tests correctly verify boundary conditions: empty objects yield no results, and null values are preserved as leaf values rather than being filtered or causing errors.
tests/utils/cronParser.test.js (1)
83-89: This review comment is based on outdated code. The current test implementation at lines 83-89, 93-98, and 102-107 already uses timezone-safe assertions and does not contain the problematic code shown in the snippet.The actual tests:
- Use explicit
Date.UTC()constructor for clarity- Assert only on timezone-independent values (
getTime()andgetMinutes())- Avoid timezone-dependent assertions (
getHours(),getDate(), etc.)- Include comments acknowledging DST/timezone concerns
No changes needed—the tests are already properly designed.
> Likely an incorrect or invalid review comment.src/modules/events/ready.js (1)
20-52: Ready handler modularization looks solid.The registration flow and feature-gate logging are clear and aligned with the new events split.
src/modules/events/errors.js (1)
16-40: Error handler extraction is well-structured.Guarding process-level handler registration and centralizing logging here is a good modularization step.
railway-app
bot
temporarily deployed
to
volvox-bot / volvox-bot-pr-240
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 24 out of 24 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
railway-app
bot
temporarily deployed
to
volvox-bot / volvox-bot-pr-240
railway-app
bot
temporarily deployed
to
volvox-bot / volvox-bot-pr-240
BillChirico
force-pushed
the
feat/code-quality-improvements
branch
from
c172185 to
a60bf12
Compare
railway-app
bot
temporarily deployed
to
volvox-bot / volvox-bot-pr-240
BillChirico
force-pushed
the
feat/code-quality-improvements
branch
from
a60bf12 to
daf4354
Compare
railway-app
bot
temporarily deployed
to
volvox-bot / volvox-bot-pr-240
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 29 out of 29 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (2)
tests/modules/events.test.js:661
- This test object defines
guildIdtwice; the first value is silently overwritten by the second. Removing the duplicate property will avoid confusion and potential lint failures.
const interaction = {
isButton: () => true,
customId: 'poll_vote_opt1',
guildId: 'g1',
user: { id: 'u1' },
guildId: 'g1',
};
tests/modules/events.test.js:686
- This test object defines
guildIdtwice; the first value is silently overwritten by the second. Removing the duplicate property will avoid confusion and potential lint failures.
const interaction = {
isButton: () => true,
customId: 'poll_vote_opt1',
guildId: 'g1',
user: { id: 'u1' },
guildId: 'g1',
replied: false,
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
railway-app
bot
temporarily deployed
to
volvox-bot / volvox-bot-pr-240
🧹 Preview Environment Cleaned UpThe Railway preview environment for this PR has been removed. Environment: |
Summary
Code quality improvements: modularize the 959-line events.js and add test coverage for utility functions.
Changes
Events Refactoring
Split
src/modules/events.js(959 lines → ~60 lines) into focused modules:events/ready.js- Client ready handler with feature loggingevents/messageCreate.js- Message processing (spam, AI triage, moderation)events/interactionCreate.js- Slash commands, buttons, modalsevents/reactions.js- Starboard and reaction rolesevents/errors.js- Error handlingevents/voiceState.js- Voice channel trackingevents/guildMemberAdd.js- Welcome messagesThe main
events.jsnow imports and re-exports all handlers for backward compatibility.New Tests
Added comprehensive test coverage:
tests/utils/cronParser.test.js(15 tests) - Cron expression parsingtests/utils/flattenToLeafPaths.test.js(11 tests) - Object flattening with prototype pollution protectiontests/api/utils/dangerousKeys.test.js(5 tests) - Dangerous key validationStats