-
Notifications
You must be signed in to change notification settings - Fork 20
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
chore: scaffold community server discord bot #220
base: main
Are you sure you want to change the base?
chore: scaffold community server discord bot #220
Conversation
📝 Walkthrough📝 WalkthroughWalkthroughThis pull request introduces a comprehensive Discord bot template with a modular and extensible architecture. The project is built using TypeScript and Discord.js, providing a structured approach to bot development. It includes various managers for handling different types of interactions like message commands, slash commands, button interactions, and more. The template offers robust configuration options, permission checks, cooldown management, and a flexible event system. Changes
Possibly related issues
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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: 49
♻️ Duplicate comments (1)
apps/discord-bot/src/structures/managers/messageCommands.ts (1)
11-11
:⚠️ Potential issueSecurity: Avoid using
file:///
protocol in dynamic importsSimilar to the EventManager, using the
file:///
protocol in dynamic imports poses security risks.
🧹 Nitpick comments (50)
apps/discord-bot/src/events/errorManager.ts (3)
1-15
: Consider implementing a more robust error handling strategyThe current error handling implementation could be enhanced with several production-ready features.
Recommendations:
- Implement error categorization to handle different types of errors differently
- Add error reporting service integration (e.g., Sentry)
- Implement recovery mechanisms for non-fatal errors
- Add rate limiting for error logging to prevent spam
- Include context information in error logs (e.g., command being executed, user info)
- Consider implementing a custom error class for bot-specific errors
Example implementation structure:
import type { ClientEvent } from '../types.js'; interface BotError extends Error { context?: { command?: string; userId?: string; guildId?: string; }; } class ErrorManager { private static errorCount = 0; private static readonly ERROR_THRESHOLD = 10; private static readonly RESET_INTERVAL = 60000; // 1 minute static initialize(): void { setInterval(() => { this.errorCount = 0; }, this.RESET_INTERVAL); } static handleError(error: BotError): void { this.errorCount++; if (this.errorCount > this.ERROR_THRESHOLD) { console.error('[CRITICAL] Error threshold exceeded'); process.exit(1); } // Log error with context console.error('[Bot Error]', { name: error.name, message: error.message, stack: error.stack, context: error.context }); // TODO: Add error reporting service integration } } export const Event: ClientEvent = { name: 'errorManager', customEvent: true, run: (): void => { ErrorManager.initialize(); process.on('unhandledRejection', (error: Error) => { ErrorManager.handleError(error as BotError); }); process.on('uncaughtException', (error: Error) => { ErrorManager.handleError(error as BotError); process.exit(1); }); }, };
1-2
: Consider using more specific error typesThe current implementation uses the generic
Error
type, which might miss specific Discord.js error properties.Consider using Discord.js specific error types:
- import type { ClientEvent } from '../types.js' + import type { ClientEvent } from '../types.js' + import { DiscordAPIError, HTTPError } from 'discord.js'
4-7
: Add JSDoc documentation for the event handlerAdding documentation will help other developers understand the purpose and behavior of this error manager.
Consider adding this documentation:
+/** + * Global error handler for the Discord bot + * Catches and processes unhandled rejections and uncaught exceptions + * to prevent the bot from crashing silently + */ export const Event: ClientEvent = { name: 'errorManager', customEvent: true, run: (): void => {apps/discord-bot/src/structures/commandOptions/onlyChannels.ts (2)
5-8
: Consider splitting complex condition for better readabilityThe compound condition could be more maintainable if split into separate checks.
- if (!command.onlyChannels || !Array.isArray(command.onlyChannels) || !message.guild) + if (!command.onlyChannels) return true + if (!Array.isArray(command.onlyChannels)) return true + if (!message.guild) return true
27-27
: Fix grammatical error in error messageThere's a grammatical error in the error message description.
- .setDescription(`The command you tried to execute cannot be ran in the current channel. Please execute the command in of these authorized channels:\n${command.onlyChannels.map(channelId => `↳ <#${channelId}>`).join('\n')}`) + .setDescription(`The command you tried to execute cannot be run in the current channel. Please execute the command in one of these authorized channels:\n${command.onlyChannels.map(channelId => `↳ <#${channelId}>`).join('\n')}`)apps/discord-bot/src/structures/commandOptions/onlyGuilds.ts (1)
5-8
: Consider improving the early return condition readabilityThe current condition combines multiple checks in one line. Consider breaking it down for better maintainability.
- if (!command.onlyGuilds || !Array.isArray(command.onlyGuilds) || !message.guild) + if (!command.onlyGuilds) return true; + if (!Array.isArray(command.onlyGuilds)) return true; + if (!message.guild) return true;apps/discord-bot/src/interactions/slashCommands/ping.ts (1)
5-5
: Enhance command description for better user experienceThe current description "Pong" is too brief. Consider providing a more informative description that explains what the command does.
- description: 'Pong', + description: 'Check the bot\'s current WebSocket latency',apps/discord-bot/src/messageCommands/ping.ts (2)
4-15
: Consider adding cooldown mechanismTo prevent potential abuse, consider implementing a cooldown mechanism for the ping command.
Would you like me to provide an implementation of a cooldown system that can be reused across commands?
1-15
: Add JSDoc documentationConsider adding JSDoc documentation to improve code maintainability and help other contributors understand the command's purpose and usage.
+/** + * @command Ping + * @description Check the bot's current WebSocket latency + * @alias pong + * @example !ping + */ export const MsgCommand: MessageCommand = {apps/discord-bot/src/structures/commandOptions/guildCooldown.ts (1)
5-8
: Consider improving function name and early return clarity.While the implementation is correct, consider these improvements:
- The function name could be more concise (e.g.,
checkGuildCooldown
)- The early return conditions could be more explicit by separating them into individual checks
-export async function guildCooldownFN( +export async function checkGuildCooldown( client: DiscordClient, message: Message | Interaction<'cached'>, command: AnyCommand, interactionType: InteractionTypeOptions ): Promise<boolean> { - if (!command.guildCooldown || Number.isNaN(command.guildCooldown) || !message.guild) + if (!command.guildCooldown) { + return true + } + if (Number.isNaN(command.guildCooldown)) { + return true + } + if (!message.guild) { return true + }apps/discord-bot/src/messageCommands/eval.ts (2)
28-39
: Clarify optional flags usage
The code removes certain flags (e.g., --silent) and modifies the snippet based on others. Consider documenting or logging the flags applied, so it’s clear why and how certain lines are removed or transformed during runtime.
60-66
: Restrict error content
In case of errors, the entire error message (potentially including stack traces) is sent to the channel. Even though restricted to owners, you may want to ensure that secret details such as environment variables or stack details are not exposed.apps/discord-bot/src/bot.ts (3)
17-33
: Consider error handling around client login
No try/catch is wrapping the login process. If the token is invalid, the script may fail silently. Consider wrapping this in a try/catch to provide explicit error logs or fallback.
35-45
: Persist cooldown management carefully
The JSON file stands as a single point of failure. If the file is removed or corrupted, cooldown data is lost. Consider a persistent store or error handling to gracefully recover from file I/O failures.
46-53
: Tests and documentation
No tests or user-facing documentation accompany this scaffold, per your PR notes. Consider adding basic test coverage for the initialization flow and step-by-step instructions for new contributors.apps/discord-bot/src/events/interactionCreate.ts (2)
8-16
: Provide user feedback if slash command not found
Currently, the code returns without notifying the user if a slash command doesn’t exist. For clarity, consider responding with a short message like “Command not recognized.”
18-26
: Autocomplete fallback
Same as slash commands, if an autocomplete is not recognized or not implemented, consider providing a fallback or log message for debugging.apps/discord-bot/src/structures/managers/slashCommands.ts (3)
38-40
: Skip reason
In case slash commands are ignored for not having a name or description, consider logging the skip reason. It helps debugging misconfigured commands.
74-76
: Context menu validation
Similar to slash commands, a short log line would help you track which context menus are ignored.
99-113
: Handle the commented out error block
The error handling is commented out. Consider reintroducing this to provide feedback if guild or global command registration fails.apps/discord-bot/src/types.ts (4)
19-47
: Extensive command options may benefit from grouping or partials.
You have a large set of optional properties for permissions, cooldown, and restrictions. Consider grouping related properties (e.g., all cooldown options) or using separate partial interfaces for improved maintainability.
49-55
: MessageCommand interface is clear.
Returning void or Promise is straightforward for message commands. Consider specifying an explicit Promise to standardize asynchronous command patterns.
57-63
: Event handling remains flexible.
The generic run signature is flexible but can obscure type safety. If feasible, refine argument typings for better IntelliSense and error checking, rather than using (...args: any[]).
96-105
: SlashCommand interface is well-formed.
Everything is consistent with standard slash commands usage. Possibly consider an additional property to specify ephemeral replies if desired in the future.apps/discord-bot/src/interactions/buttons/deleteOutput.ts (1)
1-9
: Handle potential message state errors.
The command strictly deletes the interaction’s message. Consider adding checks to handle ephemeral or already-deleted messages, as Discord may throw errors if the message is no longer available.apps/discord-bot/src/interactions/modalForms/exampleModal.ts (1)
1-10
: Simple and effective modal usage.
The reply handling is straightforward. Consider making the reply ephemeral if it only concerns the user submitting the modal to keep the channel clutter-free.apps/discord-bot/src/interactions/selectMenus/selectMenuExample.ts (1)
4-12
: Consider if this example code should be in productionThis appears to be template/example code. Consider either:
- Removing it if not needed for production
- Making it more meaningful for actual use
- Moving it to a examples/ or templates/ directory
Also, add JSDoc documentation for the exported Menu constant.
+/** + * Example select menu handler demonstrating basic Discord interaction patterns. + * @todo Replace with actual implementation or move to examples directory + */ export const Menu: SelectMenu = {apps/discord-bot/src/structures/managers/selectMenus.ts (1)
15-16
: Simplify optional chaining usageThe double optional chaining on
buttonCommand?.ignore
is redundant since we've already checked forbuttonCommand
's existence.- if (!selectMenu?.ignore && selectMenu?.name) + if (!selectMenu.ignore && selectMenu.name)apps/discord-bot/src/interactions/contextMenus/getUsername.ts (1)
9-9
: Remove unnecessary type assertionThe type assertion can be avoided by properly typing the interaction parameter in the function signature.
- run: (interaction): void => { - interaction = interaction as UserContextMenuCommandInteraction<'cached'> + run: (interaction: UserContextMenuCommandInteraction<'cached'>): Promise<void> => {apps/discord-bot/src/structures/managers/buttonCommands.ts (2)
5-18
: Consider creating a base manager class to reduce duplicationThe ButtonManager implementation is very similar to SelectMenuManager and other interaction managers. Consider creating a base manager class to reduce code duplication.
Example implementation:
abstract class BaseInteractionManager<T> { protected abstract readonly interactionType: string; async initialize(client: DiscordClient, rootPath: string): Promise<void> { const files = fileReader(`${rootPath}/interactions/${this.interactionType}`); if (!files.length) return; for (const file of files) { try { const interaction: T = (await import(file))?.export; if (!interaction) continue; if (!interaction.ignore && interaction.name) { this.addToClient(client, interaction); } } catch (error) { console.error(`Failed to load ${this.interactionType} from ${file}:`, error); } } } protected abstract addToClient(client: DiscordClient, interaction: T): void; } class ButtonManager extends BaseInteractionManager<ButtonCommand> { protected readonly interactionType = 'buttons'; protected addToClient(client: DiscordClient, button: ButtonCommand): void { client.buttonCommands?.set(button.name, button); } }
15-16
: Fix redundant optional chainingSimilar to the SelectMenuManager, the optional chaining is redundant here.
- if (!buttonCommand?.ignore && buttonCommand?.name) + if (!buttonCommand.ignore && buttonCommand.name)apps/discord-bot/src/messageCommands/callSelectMenu.ts (1)
10-26
: Consider extracting UI strings to constantsThe menu text and options are hardcoded. Consider extracting these to constants or a configuration file for easier maintenance and localization.
Example:
const MENU_STRINGS = { content: 'Cookies SelectMenu', placeholder: 'Free Cookies!', options: [{ label: 'Click for cookies!', description: 'Freeee!', value: 'CookieBox', }] } as const;apps/discord-bot/src/interactions/slashCommands/exampleModal.ts (1)
4-6
: Add JSDoc documentation for the slash commandAdd documentation to describe the command's purpose, usage, and parameters.
+/** + * Test modal command to demonstrate modal functionality + * @example + * /testmodal + */ export const Slash: SlashCommand = { name: 'testmodal', description: 'Test Modal',apps/discord-bot/src/structures/commandOptions/onlyUsers.ts (3)
9-11
: Simplify user ID checkThe current user ID check is overly complex and could be simplified.
- if (command.onlyUsers.includes(((message as Interaction<'cached'>).user ?? (message as Message).author)?.id)) { + const userId = 'user' in message ? message.user.id : message.author.id; + if (command.onlyUsers.includes(userId)) { return true }
18-29
: Enhance error message with allowed users informationThe current error message could be more helpful by listing the allowed users.
message.channel.send({ embeds: [new EmbedBuilder() .setColor('DarkRed') .setTimestamp() .setAuthor({ name: message.member?.user.globalName ?? message.member?.user.username ?? '', iconURL: message.member?.user.displayAvatarURL(), }) .setThumbnail(client.user.displayAvatarURL()) - .setDescription('The command you tried to execute couldn\'t be ran as you are not one of the authorized users.'), + .setDescription('This command is restricted to specific users.') + .addFields({ + name: 'Authorized Users', + value: command.onlyUsers.map(id => `<@${id}>`).join(', ') || 'None specified' + }), ], })
31-31
: Remove unnecessary semicolonThe semicolon after the else block is unnecessary and should be removed.
- }; + }🧰 Tools
🪛 Biome (1.9.4)
[error] 31-31: This code is unreachable
... because either this statement, ...
... this statement, ...
... this statement, ...
... this statement, ...
... or this statement will return from the function beforehand
(lint/correctness/noUnreachable)
apps/discord-bot/src/structures/commandOptions/onlyRoles.ts (2)
18-29
: Enhance error message with role namesAdd role names alongside mentions for better clarity.
message.channel.send({ embeds: [new EmbedBuilder() .setColor('DarkRed') .setTimestamp() .setAuthor({ name: message.member?.user.globalName ?? message.member?.user.username ?? '', iconURL: message.member?.user.displayAvatarURL(), }) .setThumbnail(client.user.displayAvatarURL()) - .setDescription(`The command you tried to execute couldn't be executed as you are missing one of these required roles:\n${command.onlyRoles.map((roleID: string) => `↳ <@&${roleID}>`).join('\n')}`), + .setDescription('This command requires specific roles to execute.') + .addFields({ + name: 'Required Roles', + value: command.onlyRoles.map(roleID => { + const role = message.guild?.roles.cache.get(roleID); + return `↳ <@&${roleID}>${role ? ` (${role.name})` : ''}`; + }).join('\n') || 'None specified' + }), ], })
31-31
: Remove unnecessary semicolonThe semicolon after the else block is unnecessary and should be removed.
- }; + }🧰 Tools
🪛 Biome (1.9.4)
[error] 31-32: This code is unreachable
... because either this statement, ...
... this statement, ...
... this statement, ...
... this statement, ...
... or this statement will return from the function beforehand
(lint/correctness/noUnreachable)
apps/discord-bot/src/structures/commandOptions/ownerOnly.ts (2)
24-26
: Simplify user display name retrievalThe current nested nullish coalescing could be simplified for better readability.
- name: message.member?.user.globalName ?? message.member?.user.username ?? '', - iconURL: message.member?.user.displayAvatarURL(), + name: message.member?.user.displayName ?? 'Unknown User', + iconURL: message.member?.user.displayAvatarURL() ?? client.user.defaultAvatarURL,
32-32
: Remove unnecessary semicolonThe semicolon after the closing brace is unnecessary.
- }; + }🧰 Tools
🪛 Biome (1.9.4)
[error] 32-32: This code is unreachable
... because either this statement, ...
... this statement, ...
... this statement, ...
... this statement, ...
... or this statement will return from the function beforehand
(lint/correctness/noUnreachable)
apps/discord-bot/src/events/messageCreate.ts (1)
8-8
: Consider adding rate limitingThe message handler should implement rate limiting to prevent spam and abuse.
Would you like me to provide an implementation for a rate limiting mechanism?
apps/discord-bot/src/structures/commandOptions/anyUserPermissions.ts (1)
1-31
: Add unit tests for permission checksThe PR description mentions that tests are missing. This security-critical functionality should be thoroughly tested.
Would you like me to help create unit tests for the permission checking functionality?
apps/discord-bot/src/structures/commandOptions/anyClientPermissions.ts (2)
17-28
: Add error handling for message sendingThe message sending operation could fail silently. Consider adding error handling and returning appropriate feedback.
- message.channel.send({ + await message.channel.send({ embeds: [new EmbedBuilder() // ... embed configuration ... ], - }) + }).catch(error => { + console.error('Failed to send permission error message:', error); + });
1-31
: Consider extracting common embed creation logicThe embed creation logic is duplicated across multiple permission check files. Consider extracting it into a shared utility function.
Would you like me to help create a shared utility for creating error embeds?
apps/discord-bot/src/structures/commandOptions/allUserPermissions.ts (1)
1-34
: Standardize permission types across filesThe permission types are inconsistent across files (
bigint
vsPermissionsString
). Consider standardizing the approach.Consider using
PermissionsString
consistently across all permission-related files for better type safety and maintainability.🧰 Tools
🪛 Biome (1.9.4)
[error] 33-34: This code is unreachable
... because either this statement, ...
... this statement, ...
... this statement, ...
... this statement, ...
... or this statement will return from the function beforehand
(lint/correctness/noUnreachable)
apps/discord-bot/src/structures/commandOptions/allClientPermissions.ts (2)
1-34
: Consider creating a shared permission checking moduleThere's significant code duplication across the permission checking files. Consider creating a shared module for permission checking logic.
I recommend creating a
PermissionChecker
class that encapsulates the common permission checking logic and provides methods for both client and user permission checks. This would:
- Reduce code duplication
- Centralize permission handling logic
- Make it easier to maintain and modify permission checking behavior
Would you like me to help design this shared module?
🧰 Tools
🪛 Biome (1.9.4)
[error] 33-34: This code is unreachable
... because either this statement, ...
... this statement, ...
... this statement, ...
... this statement, ...
... or this statement will return from the function beforehand
(lint/correctness/noUnreachable)
1-34
: Add unit tests for permission checkingGiven that permission checking is critical for command execution, comprehensive unit tests should be added.
Would you like me to help create unit tests for the permission checking functionality? The tests should cover:
- Various permission combinations
- Edge cases with null/undefined values
- Error message generation
- Integration with command processing
🧰 Tools
🪛 Biome (1.9.4)
[error] 33-34: This code is unreachable
... because either this statement, ...
... this statement, ...
... this statement, ...
... this statement, ...
... or this statement will return from the function beforehand
(lint/correctness/noUnreachable)
apps/discord-bot/src/structures/commandOptions/processor.ts (1)
17-17
: Add explicit return type and function name for better maintainabilityThe default export uses an anonymous arrow function without an explicit return type.
Consider this improvement:
-export default async (client: DiscordClient, message: Message | Interaction<'cached'>, command: AnyCommand, interactionType: InteractionTypeOptions) => { +export default async function processCommandOptions( + client: DiscordClient, + message: Message | Interaction<'cached'>, + command: AnyCommand, + interactionType: InteractionTypeOptions +): Promise<boolean> {apps/discord-bot/src/structures/commandOptions/globalCooldown.ts (1)
32-32
: Simplify timestamp calculation and improve readabilityThe double
Math.floor
is unnecessary and makes the code harder to read.- .setDescription(`You are currently at cooldown. Please try again in <t:${Math.floor(Math.floor(storedTime + command.globalCooldown) / 1000)}:R>.`), + const nextAvailableTime = Math.floor((storedTime + command.globalCooldown) / 1000); + .setDescription(`You are currently at cooldown. Please try again in <t:${nextAvailableTime}:R>.`),apps/discord-bot/README.md (1)
18-20
: Fix grammar and style in changelogMinor grammar issues in the changelog text.
- Added dependency of `simple-json-db` for the cooldown system as i rage quit and can't do it with `fs` myself. + Added dependency on `simple-json-db` for the cooldown system as I couldn't implement it with `fs`.🧰 Tools
🪛 LanguageTool
[uncategorized] ~19-~19: Did you mean “I”?
Context: ...ple-json-dbfor the cooldown system as i rage quit and can't do it with
fs` mys...(I_LOWERCASE_PREMIUM)
[misspelling] ~19-~19: This word is normally spelled with a hyphen.
Context: ...e-json-dbfor the cooldown system as i rage quit and can't do it with
fs` myself. - Fix...(EN_COMPOUNDS_RAGE_QUIT)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (45)
apps/discord-bot/.gitignore
(1 hunks)apps/discord-bot/LICENSE
(1 hunks)apps/discord-bot/README.md
(1 hunks)apps/discord-bot/nodemon.json
(1 hunks)apps/discord-bot/package.json
(1 hunks)apps/discord-bot/src/bot.ts
(1 hunks)apps/discord-bot/src/config.ts
(1 hunks)apps/discord-bot/src/events/errorManager.ts
(1 hunks)apps/discord-bot/src/events/interactionCreate.ts
(1 hunks)apps/discord-bot/src/events/messageCreate.ts
(1 hunks)apps/discord-bot/src/events/ready.ts
(1 hunks)apps/discord-bot/src/interactions/buttons/deleteOutput.ts
(1 hunks)apps/discord-bot/src/interactions/contextMenus/getUsername.ts
(1 hunks)apps/discord-bot/src/interactions/modalForms/exampleModal.ts
(1 hunks)apps/discord-bot/src/interactions/selectMenus/selectMenuExample.ts
(1 hunks)apps/discord-bot/src/interactions/slashCommands/exampleModal.ts
(1 hunks)apps/discord-bot/src/interactions/slashCommands/ping.ts
(1 hunks)apps/discord-bot/src/messageCommands/callSelectMenu.ts
(1 hunks)apps/discord-bot/src/messageCommands/eval.ts
(1 hunks)apps/discord-bot/src/messageCommands/ping.ts
(1 hunks)apps/discord-bot/src/structures/commandOptions/allClientPermissions.ts
(1 hunks)apps/discord-bot/src/structures/commandOptions/allUserPermissions.ts
(1 hunks)apps/discord-bot/src/structures/commandOptions/anyClientPermissions.ts
(1 hunks)apps/discord-bot/src/structures/commandOptions/anyUserPermissions.ts
(1 hunks)apps/discord-bot/src/structures/commandOptions/channelCooldown.ts
(1 hunks)apps/discord-bot/src/structures/commandOptions/globalCooldown.ts
(1 hunks)apps/discord-bot/src/structures/commandOptions/guildCooldown.ts
(1 hunks)apps/discord-bot/src/structures/commandOptions/onlyChannels.ts
(1 hunks)apps/discord-bot/src/structures/commandOptions/onlyGuilds.ts
(1 hunks)apps/discord-bot/src/structures/commandOptions/onlyRoles.ts
(1 hunks)apps/discord-bot/src/structures/commandOptions/onlyUsers.ts
(1 hunks)apps/discord-bot/src/structures/commandOptions/ownerOnly.ts
(1 hunks)apps/discord-bot/src/structures/commandOptions/processor.ts
(1 hunks)apps/discord-bot/src/structures/managers/buttonCommands.ts
(1 hunks)apps/discord-bot/src/structures/managers/events.ts
(1 hunks)apps/discord-bot/src/structures/managers/messageCommands.ts
(1 hunks)apps/discord-bot/src/structures/managers/modalForms.ts
(1 hunks)apps/discord-bot/src/structures/managers/selectMenus.ts
(1 hunks)apps/discord-bot/src/structures/managers/slashCommands.ts
(1 hunks)apps/discord-bot/src/types.ts
(1 hunks)apps/discord-bot/src/utils/fileReader.ts
(1 hunks)apps/discord-bot/tsconfig.json
(1 hunks)libs/external/aceternity/tailwind.config.ts
(1 hunks)libs/website/feature/faq/index.ts
(1 hunks)libs/website/feature/stats/index.ts
(1 hunks)
✅ Files skipped from review due to trivial changes (8)
- libs/website/feature/faq/index.ts
- apps/discord-bot/LICENSE
- apps/discord-bot/.gitignore
- libs/website/feature/stats/index.ts
- libs/external/aceternity/tailwind.config.ts
- apps/discord-bot/nodemon.json
- apps/discord-bot/tsconfig.json
- apps/discord-bot/package.json
🧰 Additional context used
🪛 Biome (1.9.4)
apps/discord-bot/src/structures/commandOptions/onlyRoles.ts
[error] 31-32: This code is unreachable
... because either this statement, ...
... this statement, ...
... this statement, ...
... this statement, ...
... or this statement will return from the function beforehand
(lint/correctness/noUnreachable)
apps/discord-bot/src/structures/commandOptions/allUserPermissions.ts
[error] 33-34: This code is unreachable
... because either this statement, ...
... this statement, ...
... this statement, ...
... this statement, ...
... or this statement will return from the function beforehand
(lint/correctness/noUnreachable)
apps/discord-bot/src/structures/commandOptions/ownerOnly.ts
[error] 32-32: This code is unreachable
... because either this statement, ...
... this statement, ...
... this statement, ...
... this statement, ...
... or this statement will return from the function beforehand
(lint/correctness/noUnreachable)
apps/discord-bot/src/structures/commandOptions/onlyGuilds.ts
[error] 31-32: This code is unreachable
... because either this statement, ...
... this statement, ...
... this statement, ...
... this statement, ...
... or this statement will return from the function beforehand
(lint/correctness/noUnreachable)
apps/discord-bot/src/structures/commandOptions/allClientPermissions.ts
[error] 33-34: This code is unreachable
... because either this statement, ...
... this statement, ...
... this statement, ...
... this statement, ...
... or this statement will return from the function beforehand
(lint/correctness/noUnreachable)
apps/discord-bot/src/structures/commandOptions/onlyChannels.ts
[error] 31-32: This code is unreachable
... because either this statement, ...
... this statement, ...
... this statement, ...
... this statement, ...
... or this statement will return from the function beforehand
(lint/correctness/noUnreachable)
apps/discord-bot/src/structures/commandOptions/onlyUsers.ts
[error] 31-31: This code is unreachable
... because either this statement, ...
... this statement, ...
... this statement, ...
... this statement, ...
... or this statement will return from the function beforehand
(lint/correctness/noUnreachable)
🪛 LanguageTool
apps/discord-bot/README.md
[uncategorized] ~19-~19: Did you mean “I”?
Context: ...ple-json-dbfor the cooldown system as i rage quit and can't do it with
fs` mys...
(I_LOWERCASE_PREMIUM)
[misspelling] ~19-~19: This word is normally spelled with a hyphen.
Context: ...e-json-dbfor the cooldown system as i rage quit and can't do it with
fs` myself. - Fix...
(EN_COMPOUNDS_RAGE_QUIT)
[style] ~78-~78: The phrase “a variety of” may be wordy. To make your writing clearer, consider replacing it.
Context: ...text menus, and modal forms. - Includes a variety of commonly used command options (not appl...
(A_VARIETY_OF)
[style] ~86-~86: Specify a number, remove phrase, or simply use “many” or “numerous”
Context: ...s may take time to refresh if there are a large number of different guild commands. - Collections...
(LARGE_NUMBER_OF)
🪛 Markdownlint (0.37.0)
apps/discord-bot/README.md
2-2: null
Images should have alternate text (alt text)
(MD045, no-alt-text)
3-3: null
Images should have alternate text (alt text)
(MD045, no-alt-text)
4-4: null
Images should have alternate text (alt text)
(MD045, no-alt-text)
5-5: null
Images should have alternate text (alt text)
(MD045, no-alt-text)
6-6: null
Images should have alternate text (alt text)
(MD045, no-alt-text)
7-7: null
Images should have alternate text (alt text)
(MD045, no-alt-text)
🔇 Additional comments (12)
apps/discord-bot/src/structures/commandOptions/onlyGuilds.ts (2)
1-4
: LGTM! Imports are well-organized and properly typed.
9-17
: Remove redundant optional chaining
The optional chaining on message.guild?.id
is unnecessary since we've already checked for !message.guild
in the early return.
- if (command.onlyGuilds.includes(message.guild?.id)) {
+ if (command.onlyGuilds.includes(message.guild.id)) {
Verify guild ID validation
Ensure that the guild IDs in command.onlyGuilds
are properly validated when commands are registered.
✅ Verification successful
Based on the context gathered, I can now generate the final response:
Optional chaining can be safely removed
The optional chaining on message.guild?.id
can indeed be safely removed since:
- The
onlyGuilds
validation is processed through a dedicated processor - The processor (
processor.ts
) runs all validations in sequence, including the guild check - The type definition in
types.ts
showsonlyGuilds
is a proper command option that's handled systematically
The suggested refactor is correct:
- if (command.onlyGuilds.includes(message.guild?.id)) {
+ if (command.onlyGuilds.includes(message.guild.id)) {
As for guild ID validation, the command registration system uses TypeScript interfaces that enforce onlyGuilds
to be an array of strings, and the validation is handled through the command options processor, which provides sufficient validation guarantees.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for command registration and guild ID validation
ast-grep --pattern 'command.onlyGuilds = $_'
Length of output: 46
Script:
#!/bin/bash
# Let's try a different approach to find guild ID validation patterns
# First, let's look for files that mention onlyGuilds
rg -l "onlyGuilds"
# Then search for any validation patterns related to guild IDs
rg "guild.*id" -A 2 -B 2
# Also search for command registration patterns
ast-grep --pattern 'new Command({
$$$
})'
Length of output: 2486
Script:
#!/bin/bash
# Let's check the Command type definition and onlyGuilds property
cat apps/discord-bot/src/types.ts
# Also check the README for any documentation about onlyGuilds
cat apps/discord-bot/README.md
# And check the command processor to understand the validation flow
cat apps/discord-bot/src/structures/commandOptions/processor.ts
Length of output: 13173
apps/discord-bot/src/structures/commandOptions/guildCooldown.ts (1)
1-4
: LGTM! Well-organized imports with proper type declarations.
The imports are clean and properly structured, with clear separation between type imports and concrete implementations.
apps/discord-bot/src/messageCommands/eval.ts (1)
41-47
: Ensure partial token leaks are protected
Currently, only exact occurrences of the client’s token are replaced with a warning. Attackers could attempt partial matches. Consider hashing or pattern matching partial tokens.
apps/discord-bot/src/bot.ts (1)
14-15
: Validate __dirname usage under ESM
Using import.meta.url with dirname is an ESM pattern. Just ensure this usage is stable across all runtime environments (like bundlers or specialized hosting).
apps/discord-bot/src/events/interactionCreate.ts (1)
28-36
: Context menus
No user feedback or logging indicates if a context menu is missing or misconfigured. A small warning log or ephemeral reply can help debugging.
apps/discord-bot/src/types.ts (6)
1-3
: Imports look solid.
All imported types appear organized with descriptive names. Ensure that all referenced dependencies (e.g., 'discord.js', 'simple-json-db') are installed and compatible versions are pinned in your package configuration.
5-17
: Module augmentation is well-structured.
Augmenting 'discord.js' with a custom 'DiscordClient' is a clean way to expand the base client. However, confirm that these optional properties are used consistently to avoid undefined at runtime when referencing them elsewhere.
65-69
: ButtonCommand, SelectMenu, and ModalForm all follow consistent design.
All three interfaces align well with the CommandOptions structure. Ensure that you handle “ownerOnly” or permissions logic consistently in your handlers if you intend to restrict usage.
Also applies to: 71-75, 77-81
83-94
: SlashCommandOptions are comprehensive.
Nested options, autocomplete, and custom choices are supported. Be mindful of command complexity when nesting many sub-options.
107-113
: ContextMenu support is neatly integrated.
Combining message and user context menus under a single interface is a great approach. Looks good as is.
115-116
: Type aliases effectively unify all command interfaces.
This helps keep the registration logic generic and simpler.
// process.on('unhandledRejection', (error: Error) => { | ||
// console.log(error) | ||
// }) | ||
// process.on('uncaughtException', (error: Error) => { | ||
// console.log(error) | ||
// }) |
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.
Uncomment and enhance error handling implementation
The error handlers are currently commented out, leaving the bot without any error management. This could lead to silent failures or unexpected crashes in production.
Consider this enhanced implementation:
- // process.on('unhandledRejection', (error: Error) => {
- // console.log(error)
- // })
- // process.on('uncaughtException', (error: Error) => {
- // console.log(error)
- // })
+ process.on('unhandledRejection', (error: Error) => {
+ console.error('[Unhandled Rejection]', {
+ name: error.name,
+ message: error.message,
+ stack: error.stack
+ });
+ // TODO: Add error reporting service integration
+ });
+
+ process.on('uncaughtException', (error: Error) => {
+ console.error('[Uncaught Exception]', {
+ name: error.name,
+ message: error.message,
+ stack: error.stack
+ });
+ // TODO: Add error reporting service integration
+
+ // Gracefully shutdown after uncaught exception
+ process.exit(1);
+ });
Committable suggestion skipped: line range outside the PR's diff.
export function onlyChannelsFN(client: DiscordClient, message: Message | Interaction<'cached'>, command: AnyCommand) { | ||
if (!command.onlyChannels || !Array.isArray(command.onlyChannels) || !message.guild) | ||
return true | ||
|
||
if (command.onlyChannels.includes(message.channel?.id)) { | ||
return true | ||
} | ||
else { | ||
if (command.returnErrors === false || command.returnOnlyChannelsError === false) | ||
return false | ||
if (!message.channel || message.channel.type !== ChannelType.GuildText) | ||
return false | ||
|
||
message.channel.send({ | ||
embeds: [new EmbedBuilder() | ||
.setColor('DarkRed') | ||
.setTimestamp() | ||
.setAuthor({ | ||
name: message.member?.user.globalName ?? message.member?.user.username ?? '', | ||
iconURL: message.member?.user.displayAvatarURL(), | ||
}) | ||
.setThumbnail(client.user.displayAvatarURL()) | ||
.setDescription(`The command you tried to execute cannot be ran in the current channel. Please execute the command in of these authorized channels:\n${command.onlyChannels.map(channelId => `↳ <#${channelId}>`).join('\n')}`), | ||
], | ||
}) | ||
return false | ||
}; | ||
} |
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.
Add comprehensive tests for permission checks
This function handles command permissions and channel restrictions, which are critical for security. Please add tests covering:
- Invalid channel IDs
- Various permission configurations
- Edge cases (null/undefined values)
- Different message/interaction types
Would you like me to help create a test suite for this function?
🧰 Tools
🪛 Biome (1.9.4)
[error] 31-32: This code is unreachable
... because either this statement, ...
... this statement, ...
... this statement, ...
... this statement, ...
... or this statement will return from the function beforehand
(lint/correctness/noUnreachable)
], | ||
}) | ||
return false | ||
}; |
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.
Remove unnecessary semicolon
The semicolon after the closing brace is unnecessary and flagged by the linter as causing unreachable code.
- };
+ }
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
}; | |
} |
🧰 Tools
🪛 Biome (1.9.4)
[error] 31-32: This code is unreachable
... because either this statement, ...
... this statement, ...
... this statement, ...
... this statement, ...
... or this statement will return from the function beforehand
(lint/correctness/noUnreachable)
if (command.onlyChannels.includes(message.channel?.id)) { | ||
return true | ||
} | ||
else { | ||
if (command.returnErrors === false || command.returnOnlyChannelsError === false) | ||
return false | ||
if (!message.channel || message.channel.type !== ChannelType.GuildText) | ||
return false | ||
|
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.
🛠️ Refactor suggestion
Simplify control flow by removing unnecessary else block
The code structure can be simplified by removing the else block and reorganizing the checks.
- if (command.onlyChannels.includes(message.channel?.id)) {
- return true
- }
- else {
- if (command.returnErrors === false || command.returnOnlyChannelsError === false)
- return false
- if (!message.channel || message.channel.type !== ChannelType.GuildText)
- return false
+ if (command.onlyChannels.includes(message.channel?.id)) return true
+
+ if (command.returnErrors === false || command.returnOnlyChannelsError === false) return false
+ if (!message.channel || message.channel.type !== ChannelType.GuildText) return false
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
if (command.onlyChannels.includes(message.channel?.id)) { | |
return true | |
} | |
else { | |
if (command.returnErrors === false || command.returnOnlyChannelsError === false) | |
return false | |
if (!message.channel || message.channel.type !== ChannelType.GuildText) | |
return false | |
if (command.onlyChannels.includes(message.channel?.id)) return true | |
if (command.returnErrors === false || command.returnOnlyChannelsError === false) return false | |
if (!message.channel || message.channel.type !== ChannelType.GuildText) return false |
message.channel.send({ | ||
embeds: [new EmbedBuilder() | ||
.setColor('DarkRed') | ||
.setTimestamp() | ||
.setAuthor({ | ||
name: message.member?.user.globalName ?? message.member?.user.username ?? '', | ||
iconURL: message.member?.user.displayAvatarURL(), | ||
}) | ||
.setThumbnail(client.user.displayAvatarURL()) | ||
.setDescription(`The command you tried to execute cannot be ran in the current guild. Please execute the command in of these authorized guilds:\n${command.onlyGuilds.map((guildID: string) => `↳ <#${guildID}>`).join('\n')}`), | ||
], | ||
}) | ||
return false | ||
}; | ||
} |
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.
Critical: Fix incorrect channel mentions in error message
The error message uses channel mentions (<#${guildID}>
) for guild IDs, which will not render correctly. This should use guild names instead.
- .setDescription(`The command you tried to execute cannot be ran in the current guild. Please execute the command in of these authorized guilds:\n${command.onlyGuilds.map((guildID: string) => `↳ <#${guildID}>`).join('\n')}`),
+ .setDescription(`The command you tried to execute cannot be ran in the current guild. Please execute the command in one of these authorized guilds:\n${command.onlyGuilds.map((guildID: string) => `↳ ${client.guilds.cache.get(guildID)?.name ?? guildID}`).join('\n')}`),
Remove unreachable semicolon
The semicolon after the return statement is unreachable and should be removed.
return false
- };
+ }
Grammar fix in error message
The error message contains a grammatical error: "cannot be ran" should be "cannot be run".
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
message.channel.send({ | |
embeds: [new EmbedBuilder() | |
.setColor('DarkRed') | |
.setTimestamp() | |
.setAuthor({ | |
name: message.member?.user.globalName ?? message.member?.user.username ?? '', | |
iconURL: message.member?.user.displayAvatarURL(), | |
}) | |
.setThumbnail(client.user.displayAvatarURL()) | |
.setDescription(`The command you tried to execute cannot be ran in the current guild. Please execute the command in of these authorized guilds:\n${command.onlyGuilds.map((guildID: string) => `↳ <#${guildID}>`).join('\n')}`), | |
], | |
}) | |
return false | |
}; | |
} | |
message.channel.send({ | |
embeds: [new EmbedBuilder() | |
.setColor('DarkRed') | |
.setTimestamp() | |
.setAuthor({ | |
name: message.member?.user.globalName ?? message.member?.user.username ?? '', | |
iconURL: message.member?.user.displayAvatarURL(), | |
}) | |
.setThumbnail(client.user.displayAvatarURL()) | |
.setDescription(`The command you tried to execute cannot be ran in the current guild. Please execute the command in one of these authorized guilds:\n${command.onlyGuilds.map((guildID: string) => `↳ ${client.guilds.cache.get(guildID)?.name ?? guildID}`).join('\n')}`), | |
], | |
}) | |
return false | |
} |
🧰 Tools
🪛 Biome (1.9.4)
[error] 31-32: This code is unreachable
... because either this statement, ...
... this statement, ...
... this statement, ...
... this statement, ...
... or this statement will return from the function beforehand
(lint/correctness/noUnreachable)
let allSlashCommands: Array<string> = fileReader(`${rootPath}/interactions/slashCommands`) | ||
allSlashCommands = await allSlashCommands.reduce(async (array: any, slash: string): Promise<Array<string>> => { | ||
const command: SlashCommand | undefined = (await import(`file:///${slash}`))?.Slash | ||
|
||
if (command?.ignore || !command?.name) | ||
return array | ||
else return (await array).concat(slash) | ||
}, []) |
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.
Improve error handling and type safety in slash commands loading
The slash commands loading logic has several potential issues:
- Using
any
type for the accumulator weakens type safety - No error handling for failed imports
- Potential file path issues with
file:///
prefix
Consider this safer implementation:
- allSlashCommands = await allSlashCommands.reduce(async (array: any, slash: string): Promise<Array<string>> => {
- const command: SlashCommand | undefined = (await import(`file:///${slash}`))?.Slash
+ allSlashCommands = await allSlashCommands.reduce(async (array: Promise<string[]>, slash: string): Promise<string[]> => {
+ const acc = await array;
+ try {
+ const command: SlashCommand | undefined = (await import(slash))?.Slash
+ if (command?.ignore || !command?.name) return acc;
+ return acc.concat(slash);
+ } catch (error) {
+ console.error(`Failed to load slash command from ${slash}:`, error);
+ return acc;
+ }
+ }, Promise.resolve([]))
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
let allSlashCommands: Array<string> = fileReader(`${rootPath}/interactions/slashCommands`) | |
allSlashCommands = await allSlashCommands.reduce(async (array: any, slash: string): Promise<Array<string>> => { | |
const command: SlashCommand | undefined = (await import(`file:///${slash}`))?.Slash | |
if (command?.ignore || !command?.name) | |
return array | |
else return (await array).concat(slash) | |
}, []) | |
let allSlashCommands: Array<string> = fileReader(`${rootPath}/interactions/slashCommands`) | |
allSlashCommands = await allSlashCommands.reduce(async (array: Promise<string[]>, slash: string): Promise<string[]> => { | |
const acc = await array; | |
try { | |
const command: SlashCommand | undefined = (await import(slash))?.Slash | |
if (command?.ignore || !command?.name) return acc; | |
return acc.concat(slash); | |
} catch (error) { | |
console.error(`Failed to load slash command from ${slash}:`, error); | |
return acc; | |
} | |
}, Promise.resolve([])) |
let allContextMenus: Array<string> = fileReader(`${rootPath}/interactions/contextMenus`) | ||
allContextMenus = await allContextMenus.reduce(async (array: any, context: string): Promise<Array<string>> => { | ||
const command: ContextMenu | undefined = (await import(`file:///${context}`))?.Context | ||
|
||
if (command?.ignore || !command?.name || !command?.type) | ||
return array | ||
else return (await array).concat(context) | ||
}, []) |
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.
Apply consistent error handling pattern to context menus loading
The context menus loading has the same issues as the slash commands loading.
Apply similar improvements:
- allContextMenus = await allContextMenus.reduce(async (array: any, context: string): Promise<Array<string>> => {
- const command: ContextMenu | undefined = (await import(`file:///${context}`))?.Context
+ allContextMenus = await allContextMenus.reduce(async (array: Promise<string[]>, context: string): Promise<string[]> => {
+ const acc = await array;
+ try {
+ const command: ContextMenu | undefined = (await import(context))?.Context
+ if (command?.ignore || !command?.name || !command?.type) return acc;
+ return acc.concat(context);
+ } catch (error) {
+ console.error(`Failed to load context menu from ${context}:`, error);
+ return acc;
+ }
+ }, Promise.resolve([]))
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
let allContextMenus: Array<string> = fileReader(`${rootPath}/interactions/contextMenus`) | |
allContextMenus = await allContextMenus.reduce(async (array: any, context: string): Promise<Array<string>> => { | |
const command: ContextMenu | undefined = (await import(`file:///${context}`))?.Context | |
if (command?.ignore || !command?.name || !command?.type) | |
return array | |
else return (await array).concat(context) | |
}, []) | |
let allContextMenus: Array<string> = fileReader(`${rootPath}/interactions/contextMenus`) | |
allContextMenus = await allContextMenus.reduce(async (array: Promise<string[]>, context: string): Promise<string[]> => { | |
const acc = await array; | |
try { | |
const command: ContextMenu | undefined = (await import(context))?.Context | |
if (command?.ignore || !command?.name || !command?.type) return acc; | |
return acc.concat(context); | |
} catch (error) { | |
console.error(`Failed to load context menu from ${context}:`, error); | |
return acc; | |
} | |
}, Promise.resolve([])) |
if ((client.messageCommands?.size ?? 0) > 0) { | ||
// console.log(t.bold.red.toFunction()('[MessageCommands] ') + t.bold.cyan.toFunction()(`Loaded ${(client.messageCommands?.size ?? 0)} MessageCommands with ${t.bold.white.toFunction()(`${client.messageCommands_Aliases?.size} Aliases`)}.`)) | ||
if ((client.events?.size ?? 0) > 0) { | ||
// console.log(t.bold.yellow.toFunction()('[Events] ') + t.bold.magenta.toFunction()(`Loaded ${(client.events?.size ?? 0)} Events.`)) | ||
if ((client.buttonCommands?.size ?? 0) > 0) { | ||
// console.log(t.bold.brightGreen.toFunction()('[ButtonCommands] ') + t.bold.brightYellow.toFunction()(`Loaded ${(client.buttonCommands?.size ?? 0)} Buttons.`)) | ||
if ((client.selectMenus?.size ?? 0) > 0) { | ||
// console.log(t.bold.red.toFunction()('[SelectMenus] ') + t.bold.brightBlue.toFunction()(`Loaded ${(client.selectMenus?.size ?? 0)} SelectMenus.`)) | ||
if ((client.modalForms?.size ?? 0) > 0) { | ||
// console.log(t.bold.brightCyan.toFunction()('[ModalForms] ') + t.bold.brightYellow.toFunction()(`Loaded ${(client.modalForms?.size ?? 0)} Modals.`)) | ||
if (allSlashCommands?.length > 0) { | ||
// console.log(t.bold.magenta.toFunction()('[SlashCommands] ') + t.bold.white.toFunction()(`Loaded ${allSlashCommands.length} SlashCommands.`)) | ||
if (allContextMenus?.length > 0) { | ||
// console.log(t.bold.magenta.toFunction()('[ContextMenus] ') + t.bold.white.toFunction()(`Loaded ${allContextMenus.length} ContextMenus.`)) | ||
} | ||
} | ||
} | ||
} | ||
} | ||
} | ||
} |
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.
🛠️ Refactor suggestion
Simplify deeply nested conditional logging
The current nested if-statements make the code hard to maintain and understand.
Consider flattening the structure:
- if ((client.messageCommands?.size ?? 0) > 0) {
- // console.log...
- if ((client.events?.size ?? 0) > 0) {
- // console.log...
- if ((client.buttonCommands?.size ?? 0) > 0) {
- // ...more nesting
- }
- }
- }
+ const stats = {
+ messageCommands: client.messageCommands?.size ?? 0,
+ events: client.events?.size ?? 0,
+ buttonCommands: client.buttonCommands?.size ?? 0,
+ selectMenus: client.selectMenus?.size ?? 0,
+ modalForms: client.modalForms?.size ?? 0,
+ slashCommands: allSlashCommands?.length ?? 0,
+ contextMenus: allContextMenus?.length ?? 0
+ };
+
+ Object.entries(stats).forEach(([type, count]) => {
+ if (count > 0) {
+ // console.log(`[${type}] Loaded ${count} ${type}`);
+ }
+ });
Committable suggestion skipped: line range outside the PR's diff.
## Notes | ||
|
||
- Recommended Node.js version: 16 and above. | ||
- Global slash commands and context menus may take time to refresh as it is controlled by Discord. | ||
- Guild commands may take time to refresh if there are a large number of different guild commands. | ||
- Collections where command and event data is stored and used: | ||
- `<Client>.messageCommands`: Message commands cache | ||
- `<Client>.messageCommands_Aliases`: Message command aliases cache | ||
- `<Client>.events`: Client events cache | ||
- `<Client>.buttonCommands`: Button interactions cache | ||
- `<Client>.selectMenus`: Select menu interactions cache | ||
- `<Client>.modalForms`: Modal form interactions cache | ||
- `<Client>.slashCommands`: Slash commands cache | ||
- `<Client>.contextMenus`: ContextMenus commands cache | ||
|
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.
🛠️ Refactor suggestion
Add documentation for community server integration
The README lacks information about the specific purpose of this bot in the community server context.
Consider adding a new section:
## Community Server Integration
This bot serves as a foundation for linking server roles to third-party applications. Key features include:
- Role management integration capabilities
- Third-party application linking support
- Portal integration (see issue #217)
For development and contribution guidelines specific to the community server features, see [CONTRIBUTING.md].
🧰 Tools
🪛 LanguageTool
[style] ~86-~86: Specify a number, remove phrase, or simply use “many” or “numerous”
Context: ...s may take time to refresh if there are a large number of different guild commands. - Collections...
(LARGE_NUMBER_OF)
<p align="center"> | ||
<img src="https://media.discordapp.net/attachments/774290264764055582/1093484780525469757/A_banner_for_a_discord_bots_template_made_using_discord.js.png" height="200" width="400"><br> | ||
<img src="https://img.shields.io/badge/version-1.0.7-05122A?style=for-the-badge"> | ||
<a href="https://discord.gg/VStdRr8nP2"><img src="https://img.shields.io/badge/discord-invite-5865f2?style=for-the-badge&logo=discord&logoColor=white"></a> | ||
<img src="https://img.shields.io/github/issues/RileCraft/DiscordBot-Template-ts.svg?style=for-the-badge"> | ||
<img src="https://img.shields.io/github/forks/RileCraft/DiscordBot-Template-ts.svg?style=for-the-badge"> | ||
<img src="https://img.shields.io/github/stars/RileCraft/DiscordBot-Template-ts.svg?style=for-the-badge"> | ||
</p> |
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.
Add alt text to images for accessibility
Images should have descriptive alt text for accessibility.
Update the image tags:
- <img src="https://media.discordapp.net/attachments/774290264764055582/1093484780525469757/A_banner_for_a_discord_bots_template_made_using_discord.js.png" height="200" width="400"><br>
- <img src="https://img.shields.io/badge/version-1.0.7-05122A?style=for-the-badge">
+ <img src="https://media.discordapp.net/attachments/774290264764055582/1093484780525469757/A_banner_for_a_discord_bots_template_made_using_discord.js.png" alt="Discord Bot Template Banner" height="200" width="400"><br>
+ <img src="https://img.shields.io/badge/version-1.0.7-05122A?style=for-the-badge" alt="Version 1.0.7">
Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Markdownlint (0.37.0)
2-2: null
Images should have alternate text (alt text)
(MD045, no-alt-text)
3-3: null
Images should have alternate text (alt text)
(MD045, no-alt-text)
4-4: null
Images should have alternate text (alt text)
(MD045, no-alt-text)
5-5: null
Images should have alternate text (alt text)
(MD045, no-alt-text)
6-6: null
Images should have alternate text (alt text)
(MD045, no-alt-text)
7-7: null
Images should have alternate text (alt text)
(MD045, no-alt-text)
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: 8
🧹 Nitpick comments (8)
apps/discord-bot/template-server/src/storage.ts (1)
1-1
: Consider using persistent storage instead of in-memory MapUsing an in-memory Map for token storage has several limitations:
- Data is lost when the server restarts
- Doesn't scale across multiple instances
- No persistence guarantees
Consider using a persistent storage solution like Redis or a database for token storage in production. This would provide:
- Data persistence across restarts
- Horizontal scalability
- Better security features
- Built-in encryption capabilities
Would you like me to provide an example implementation using Redis or a database?
apps/discord-bot/template-server/src/discord.ts (2)
36-60
: Add clearer error-handling strategies.
The error is thrown when the response is not okay, but consider adding fallback logic or retry attempts to mitigate transient failures. This can be helpful if Discord’s API has intermittent issues.
119-126
: Avoid hardcoding platform_name.
"Example Linked Role Discord Bot" is hardcoded. If you intend to change the bot branding or re-purpose the same code for multiple bots, consider placing this value in configuration.apps/discord-bot/template-server/src/register.ts (1)
7-54
: Check numeric type definitions for metadata.
The numeric type integers (e.g., 2, 7) have domain-specific meanings in Discord’s role-connection metadata schema. To improve maintainability, define constants or an enum referencing official documentation, making the code self-documenting and less prone to mistakes if these values change.apps/discord-bot/template-server/src/server.ts (2)
59-62
: Confirm state verification logic and error responses.
The 403 status code is appropriate for invalid states, but consider returning an explanatory message or a redirect to clarify for a user who might accidentally end up here.
64-65
: Handle null or malformed OAuth code.
If the user arrives without a valid OAuth code, the request to get tokens will fail with a less specific error. Consider explicitly checking for a missing or invalid code before calling getOAuthTokens.apps/discord-bot/template-server/README.md (2)
35-104
: Add language specifiers to code blocksFor better syntax highlighting and documentation consistency, add language specifiers to the code blocks. For example:
-``` +```bash git clone https://github.com/discord/linked-roles-sample.gitThis applies to all code blocks in the setup and running sections. <details> <summary>🧰 Tools</summary> <details> <summary>🪛 Markdownlint (0.37.0)</summary> 74-74: null Dollar signs used before commands without showing output (MD014, commands-show-output) --- 80-80: null Dollar signs used before commands without showing output (MD014, commands-show-output) --- 90-90: null Dollar signs used before commands without showing output (MD014, commands-show-output) --- 35-35: null Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 41-41: null Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 50-50: null Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 60-60: null Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 73-73: null Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 79-79: null Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 89-89: null Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 95-95: null Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> </details> --- `114-118`: **Format URL as markdown link and add internal documentation references** Please format the bare URL as a proper markdown link and consider adding references to internal documentation: ```diff - Read **[the tutorial](https://discord.com/developers/docs/tutorials/configuring-app-metadata-for-linked-roles)** for in-depth information. -- Browse https://github.com/JustinBeckwith/fitbit-discord-bot/ for a more in-depth example using the Fitbit API +- Browse [Fitbit Discord Bot Example](https://github.com/JustinBeckwith/fitbit-discord-bot/) for a more in-depth example using the Fitbit API - Join the **[Discord Developers server](https://discord.gg/discord-developers)** to ask questions about the API, attend events hosted by the Discord API team, and interact with other devs. +- Check our [Internal Bot Development Guide](../docs/bot-development.md) for team-specific guidelines and best practices
🧰 Tools
🪛 Markdownlint (0.37.0)
117-117: null
Bare URL used(MD034, no-bare-urls)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
apps/discord-bot/template-server/package-lock.json
is excluded by!**/package-lock.json
📒 Files selected for processing (8)
apps/discord-bot/template-server/.gitignore
(1 hunks)apps/discord-bot/template-server/README.md
(1 hunks)apps/discord-bot/template-server/package.json
(1 hunks)apps/discord-bot/template-server/src/config.ts
(1 hunks)apps/discord-bot/template-server/src/discord.ts
(1 hunks)apps/discord-bot/template-server/src/register.ts
(1 hunks)apps/discord-bot/template-server/src/server.ts
(1 hunks)apps/discord-bot/template-server/src/storage.ts
(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- apps/discord-bot/template-server/.gitignore
- apps/discord-bot/template-server/package.json
🧰 Additional context used
🪛 LanguageTool
apps/discord-bot/template-server/README.md
[style] ~8-~8: Consider removing “of” to be more concise
Context: ...ole-discord-bot) ## Project structure All of the files for the project are on the left-h...
(ALL_OF_THE)
🪛 Markdownlint (0.37.0)
apps/discord-bot/template-server/README.md
74-74: null
Dollar signs used before commands without showing output
(MD014, commands-show-output)
80-80: null
Dollar signs used before commands without showing output
(MD014, commands-show-output)
90-90: null
Dollar signs used before commands without showing output
(MD014, commands-show-output)
117-117: null
Bare URL used
(MD034, no-bare-urls)
11-11: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
35-35: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
41-41: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
50-50: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
60-60: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
73-73: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
79-79: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
89-89: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
95-95: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (4)
apps/discord-bot/template-server/src/storage.ts (1)
1-9
:
Add security measures for token storage
The current implementation stores sensitive tokens without any security measures.
Consider implementing these security measures:
- Encrypt tokens before storage
- Implement token rotation
- Add token expiration checks
- Implement proper logging for security events
- Add rate limiting for token operations
Would you like me to provide an example implementation with these security measures?
Let's check if there are any security-related configurations or encryption utilities in the codebase:
apps/discord-bot/template-server/src/discord.ts (1)
19-30
: Consider validating or white-listing OAuth state.
While using a randomly generated state is good practice for preventing CSRF attacks, make sure you have a clear process to validate or track legitimate states beyond just storing them in cookies (e.g., storing in a database if there's high security risk).
apps/discord-bot/template-server/src/server.ts (1)
107-134
: Implement robust error handling for external data fetch failures.
When external data fetch (e.g., from cookie data or a third-party service) fails, the current approach sets metadata to an empty object. This is functional but might need logging or user-friendly error notifications if the user’s data is crucial.
apps/discord-bot/template-server/README.md (1)
36-36
:
Update repository URL to match the current project
The clone URL currently points to discord/linked-roles-sample
. This should be updated to point to your organization's repository:
-git clone https://github.com/discord/linked-roles-sample.git
+git clone https://github.com/cuhacking/2025.git
Likely invalid or redundant comment.
export async function storeDiscordTokens(userId: string, tokens: any) { | ||
await store.set(`discord-${userId}`, tokens) | ||
} |
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.
Improve type safety and error handling for token storage
Several improvements needed for production readiness:
any
type should be replaced with a proper interface- Unnecessary
async/await
usage - Missing error handling
- No validation for userId or tokens
Consider this improved implementation:
+ interface DiscordTokens {
+ access_token: string;
+ refresh_token: string;
+ expires_at: number;
+ }
+
- export async function storeDiscordTokens(userId: string, tokens: any) {
- await store.set(`discord-${userId}`, tokens)
+ export function storeDiscordTokens(userId: string, tokens: DiscordTokens): void {
+ if (!userId?.trim()) {
+ throw new Error('Invalid userId provided');
+ }
+ if (!tokens?.access_token || !tokens?.refresh_token) {
+ throw new Error('Invalid tokens object');
+ }
+ store.set(`discord-${userId}`, tokens);
+ }
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
export async function storeDiscordTokens(userId: string, tokens: any) { | |
await store.set(`discord-${userId}`, tokens) | |
} | |
interface DiscordTokens { | |
access_token: string; | |
refresh_token: string; | |
expires_at: number; | |
} | |
export function storeDiscordTokens(userId: string, tokens: DiscordTokens): void { | |
if (!userId?.trim()) { | |
throw new Error('Invalid userId provided'); | |
} | |
if (!tokens?.access_token || !tokens?.refresh_token) { | |
throw new Error('Invalid tokens object'); | |
} | |
store.set(`discord-${userId}`, tokens); | |
} |
const store = new Map() | ||
|
||
export async function storeDiscordTokens(userId: string, tokens: any) { | ||
await store.set(`discord-${userId}`, tokens) | ||
} | ||
|
||
export async function getDiscordTokens(userId: string) { | ||
return store.get(`discord-${userId}`) | ||
} |
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.
💡 Codebase verification
Based on the package.json files and previous test file searches, I can now generate the final response:
Add unit tests for token storage operations
The Discord bot app currently lacks a testing setup, while the project has Vitest available in the root dependencies. You'll need to:
- Set up a test directory in the Discord bot app
- Configure Vitest for the Discord bot
- Create storage test file covering:
- Token storage/retrieval operations
- Input validation
- Error handling
- Security measures
Example test file location: apps/discord-bot/src/__tests__/storage.test.ts
🔗 Analysis chain
Add unit tests for token storage operations
Based on the PR objectives, tests haven't been added yet. Token storage is critical functionality that requires thorough testing.
Would you like me to generate comprehensive unit tests covering:
- Token storage success/failure cases
- Token retrieval success/failure cases
- Input validation
- Error handling
- Security measures
Let's check the testing setup in the project:
Let me check if there are any existing tests specifically for the Discord bot functionality.
Let me check if there's a testing framework setup in the discord-bot app or its parent directory.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Look for test files and testing utilities
fd -e test.ts -e spec.ts
# Check for testing framework configurations
fd -e jest.config -e vitest.config
Length of output: 309
Script:
#!/bin/bash
# Look for Discord-related test files that might have been missed
rg -l "discord" --type ts $(fd -e test.ts -e spec.ts)
# Check if there's a test directory in the discord-bot app
fd . apps/discord-bot -t d | grep -i "test"
# Look for any testing utilities or mocks related to storage
rg -l "storage" --type ts $(fd -e test.ts -e spec.ts)
Length of output: 674
Script:
#!/bin/bash
# Look for package.json in discord-bot app to check testing dependencies
cat apps/discord-bot/package.json
# Check for any test scripts or testing configurations in the root package.json
cat package.json
Length of output: 6297
* token. Check if the access token has expired, and if it has, use the | ||
* refresh token to acquire a new, fresh access token. | ||
*/ | ||
export async function getAccessToken(userId: any, tokens: any) { |
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.
🛠️ Refactor suggestion
Use a more specific type instead of 'any' for userId.
The parameter 'userId: any' can introduce type safety issues. Consider defining a more specific type, such as string
or a custom user type, to avoid potential runtime errors.
const config = { | ||
DISCORD_TOKEN: process.env.DISCORD_TOKEN, | ||
DISCORD_CLIENT_ID: process.env.DISCORD_CLIENT_ID, | ||
DISCORD_CLIENT_SECRET: process.env.DISCORD_CLIENT_SECRET, | ||
DISCORD_REDIRECT_URI: process.env.DISCORD_REDIRECT_URI, | ||
COOKIE_SECRET: process.env.COOKIE_SECRET, | ||
} |
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.
🛠️ Refactor suggestion
Add runtime validation of environment variables.
While loading them is straightforward, missing or invalid environment variables can cause subtler bugs downstream. A small validation step here to confirm values aren’t undefined or empty strings can save time diagnosing errors.
``` | ||
$ node server.js | ||
``` |
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.
Add TypeScript-specific running instructions
Since this is a TypeScript project, please add compilation and development mode instructions:
+### Development
+
+To run in development mode with automatic recompilation:
+```bash
+npm run dev
+```
+
+### Production
+
+To build and run for production:
+```bash
+npm run build
+npm start
🧰 Tools
🪛 Markdownlint (0.37.0)
74-74: null
Dollar signs used before commands without showing output
(MD014, commands-show-output)
73-73: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
|
||
> ❇️ A version of this code is also hosted [on Glitch 🎏](https://glitch.com/edit/#!/linked-role-discord-bot) | ||
|
||
## Project structure |
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.
Add testing documentation section
As mentioned in the PR objectives, testing documentation is required. Please add a new section that covers:
- Test setup instructions
- How to run tests
- How to write new tests
- Test coverage requirements
Example structure:
## Testing
### Setup
Instructions for setting up the test environment...
### Running Tests
```bash
npm test
npm run test:watch # for development
npm run test:coverage # to generate coverage report
Writing Tests
Guidelines for writing new tests...
Coverage Requirements
Minimum coverage thresholds and requirements...
<!-- This is an auto-generated comment by CodeRabbit -->
├── assets -> Images used in this tutorial | ||
├── src | ||
│ ├── config.js -> Parsing of local configuration | ||
│ ├── discord.js -> Discord specific auth & API wrapper | ||
│ ├── register.js -> Tool to register the metadata schema | ||
│ ├── server.js -> Main entry point for the application | ||
│ ├── storage.js -> Provider for storing OAuth2 tokens | ||
├── .env -> your credentials and IDs | ||
├── .gitignore | ||
├── package.json | ||
└── README.md |
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.
Update project structure to reflect TypeScript implementation
The project structure shows .js
files, but according to the PR objectives, this is a TypeScript project. Please update the file extensions to .ts
for accuracy:
├── assets -> Images used in this tutorial
├── src
-│ ├── config.js -> Parsing of local configuration
-│ ├── discord.js -> Discord specific auth & API wrapper
-│ ├── register.js -> Tool to register the metadata schema
-│ ├── server.js -> Main entry point for the application
-│ ├── storage.js -> Provider for storing OAuth2 tokens
+│ ├── config.ts -> Parsing of local configuration
+│ ├── discord.ts -> Discord specific auth & API wrapper
+│ ├── register.ts -> Tool to register the metadata schema
+│ ├── server.ts -> Main entry point for the application
+│ ├── storage.ts -> Provider for storing OAuth2 tokens
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
├── assets -> Images used in this tutorial | |
├── src | |
│ ├── config.js -> Parsing of local configuration | |
│ ├── discord.js -> Discord specific auth & API wrapper | |
│ ├── register.js -> Tool to register the metadata schema | |
│ ├── server.js -> Main entry point for the application | |
│ ├── storage.js -> Provider for storing OAuth2 tokens | |
├── .env -> your credentials and IDs | |
├── .gitignore | |
├── package.json | |
└── README.md | |
├── assets -> Images used in this tutorial | |
├── src | |
│ ├── config.ts -> Parsing of local configuration | |
│ ├── discord.ts -> Discord specific auth & API wrapper | |
│ ├── register.ts -> Tool to register the metadata schema | |
│ ├── server.ts -> Main entry point for the application | |
│ ├── storage.ts -> Provider for storing OAuth2 tokens | |
├── .env -> your credentials and IDs | |
├── .gitignore | |
├── package.json | |
└── README.md |
- git clone https://github.com/RileCraft/DiscordBot-Template-TS.git apps/discord-bot - delete .git & .github - tidy up package.json - pnpm lint:fix
- acts as the 3rd party app for the bot to communicate with
ee33f55
to
d63677a
Compare
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: 32
♻️ Duplicate comments (8)
apps/discord-bot/src/structures/commandOptions/onlyGuilds.ts (1)
18-32
:⚠️ Potential issueFix error message formatting and grammar issues
The error handling has several issues that need to be addressed:
.setDescription( - `The command you tried to execute cannot be ran in the current guild. Please execute the command in of these authorized guilds:\n${command.onlyGuilds.map((guildID: string) => `↳ <#${guildID}>`).join('\n')}`, + `The command you tried to execute cannot be run in the current guild. Please execute the command in one of these authorized guilds:\n${command.onlyGuilds.map((guildID: string) => `↳ ${client.guilds.cache.get(guildID)?.name ?? guildID}`).join('\n')}`, ) ], }) - return false - }; + return false + }Changes:
- Fixed grammar: "cannot be ran" → "cannot be run"
- Fixed typo: "in of these" → "in one of these"
- Replaced channel mentions with guild names
- Removed unreachable semicolon
🧰 Tools
🪛 Biome (1.9.4)
[error] 31-32: This code is unreachable
... because either this statement, ...
... this statement, ...
... this statement, ...
... this statement, ...
... or this statement will return from the function beforehand
(lint/correctness/noUnreachable)
apps/discord-bot/src/events/errorManager.ts (1)
8-13
:⚠️ Potential issueUncomment and enhance error handling implementation
The error handlers are currently commented out, leaving the bot without any error management. This could lead to silent failures or unexpected crashes in production.
Additional enhancement: Consider implementing a custom error handling class to centralize error management and add structured logging:
class ErrorManager { private static formatError(error: Error) { return { name: error.name, message: error.message, stack: error.stack, timestamp: new Date().toISOString() }; } static handleUnhandledRejection(error: Error) { console.error('[Unhandled Rejection]', this.formatError(error)); // TODO: Add error reporting service integration } static handleUncaughtException(error: Error) { console.error('[Uncaught Exception]', this.formatError(error)); // TODO: Add error reporting service integration // Gracefully shutdown after uncaught exception process.exit(1); } } process.on('unhandledRejection', ErrorManager.handleUnhandledRejection); process.on('uncaughtException', ErrorManager.handleUncaughtException);apps/discord-bot/src/structures/managers/events.ts (1)
10-23
:⚠️ Potential issueEnhance event registration with proper error handling and type safety
Combining previous review suggestions with additional improvements for robust event handling.
- for (const event of eventFiles) { + for (const eventFile of eventFiles) { + try { + // Validate event file path + if (!eventFile.endsWith('.js') && !eventFile.endsWith('.ts')) { + console.warn(`Skipping invalid event file: ${eventFile}`); + continue; + } + - const clientEvent: ClientEvent = (await import(`file:///${event}`))?.Event + const clientEvent: ClientEvent = (await import(eventFile))?.Event + + // Validate event structure + if (!clientEvent || typeof clientEvent !== 'object') { + console.warn(`Invalid event export in ${eventFile}`); + continue; + } + if (clientEvent.ignore) continue client.events?.set(clientEvent.name, clientEvent) if (clientEvent.customEvent) - clientEvent.run(client) + await clientEvent.run(client) else if (clientEvent.name && clientEvent.runOnce) - client.once(clientEvent.name, (...args: unknown[]) => clientEvent.run(...args, client)) + client.once(clientEvent.name, async (...args: Parameters<typeof clientEvent.run>) => { + try { + await clientEvent.run(...args, client); + } catch (error) { + console.error(`Error in event ${clientEvent.name}:`, error); + } + }) else if (clientEvent.name) - client.on(clientEvent.name, (...args: unknown[]) => clientEvent.run(...args, client)) + client.on(clientEvent.name, async (...args: Parameters<typeof clientEvent.run>) => { + try { + await clientEvent.run(...args, client); + } catch (error) { + console.error(`Error in event ${clientEvent.name}:`, error); + } + }) + } catch (error) { + console.error(`Failed to load event from ${eventFile}:`, error); + } };apps/discord-bot/src/events/messageCreate.ts (2)
11-28
:⚠️ Potential issueReplace forEach with for...of for async operations
Using
forEach
with async operations can lead to race conditions and doesn't properly handle promise rejections.- PREFIX.forEach(async (botPrefix: string) => { + for (const botPrefix of PREFIX) { + try { if (!message.content.startsWith(botPrefix)) - return + continue; // ... rest of the code ... if (processedCommandOptionsChecker) await command.run(client, message, args) + } catch (error) { + console.error('Error processing command:', error); + // Consider sending an error message to the channel + } - }) + }
18-18
:⚠️ Potential issueAdd input validation for command arguments
The current implementation splits arguments without validation, which could lead to security issues.
apps/discord-bot/src/structures/commandOptions/anyUserPermissions.ts (2)
8-10
:⚠️ Potential issueImprove permission check error handling
The permission check could fail silently if
message.member
is null.
26-26
: 🛠️ Refactor suggestionEnhance permission display in error message
The current implementation shows raw permission bits, which are not user-friendly.
apps/discord-bot/src/structures/commandOptions/anyClientPermissions.ts (1)
8-10
: 🛠️ Refactor suggestionImprove permission check robustness
The permission check could be simplified and made more robust by using optional chaining consistently.
🧹 Nitpick comments (33)
apps/discord-bot/template-server/src/storage.ts (1)
1-1
: Consider a more robust storage solution for production useUsing an in-memory Map for token storage has several limitations:
- Tokens are lost when the server restarts
- Not scalable across multiple instances
- No persistence
Consider implementing a storage interface that can be backed by different implementations:
interface TokenStorage { set(key: string, value: DiscordTokens): Promise<void>; get(key: string): Promise<DiscordTokens | null>; } class InMemoryStorage implements TokenStorage { private store: Map<string, DiscordTokens> = new Map(); async set(key: string, value: DiscordTokens): Promise<void> { this.store.set(key, value); } async get(key: string): Promise<DiscordTokens | null> { return this.store.get(key) || null; } }This allows for easy switching between storage backends (Redis, PostgreSQL, etc.) while maintaining type safety.
apps/discord-bot/src/structures/commandOptions/onlyChannels.ts (2)
5-8
: Add additional parameter validationWhile the current validation is good, consider adding these safety checks:
- Validate that
client
is not null- Ensure
command
object has required propertiesexport function onlyChannelsFN(client: DiscordClient, message: Message | Interaction<'cached'>, command: AnyCommand) { + if (!client || !message || !command) return false; if (!command.onlyChannels || !Array.isArray(command.onlyChannels) || !message.guild) return true
18-29
: Consider limiting the channel list lengthThe channel list in the error message could become very long if there are many authorized channels. Consider:
- Limiting the number of channels shown
- Adding a "and X more..." suffix if truncated
- .setDescription(`The command you tried to execute cannot be ran in the current channel. Please execute the command in of these authorized channels:\n${command.onlyChannels.map(channelId => `↳ <#${channelId}>`).join('\n')}`) + .setDescription(`The command you tried to execute cannot be ran in the current channel. Please execute the command in of these authorized channels:\n${command.onlyChannels.slice(0, 5).map(channelId => `↳ <#${channelId}>`).join('\n')}${command.onlyChannels.length > 5 ? `\n...and ${command.onlyChannels.length - 5} more channels` : ''}`)apps/discord-bot/src/structures/commandOptions/onlyGuilds.ts (3)
5-8
: Consider improving readability of the validation conditionThe compound condition could be more readable if split into multiple lines.
- if (!command.onlyGuilds || !Array.isArray(command.onlyGuilds) || !message.guild) + if ( + !command.onlyGuilds || + !Array.isArray(command.onlyGuilds) || + !message.guild + )
9-12
: Remove redundant optional chaining and unnecessary else blockThe code can be simplified:
- Optional chaining on
message.guild?.id
is redundant since we already checked!message.guild
- The
else
block is unnecessary since the previous block returns- if (command.onlyGuilds.includes(message.guild?.id)) { + if (command.onlyGuilds.includes(message.guild.id)) { return true - } - else {
1-32
: Consider architectural improvements
- The error messages should be externalized for localization support.
- As mentioned in the PR objectives, unit tests should be added to verify the behavior of this function.
Would you like me to:
- Propose a structure for implementing localization?
- Generate unit tests for this function?
🧰 Tools
🪛 Biome (1.9.4)
[error] 31-32: This code is unreachable
... because either this statement, ...
... this statement, ...
... this statement, ...
... this statement, ...
... or this statement will return from the function beforehand
(lint/correctness/noUnreachable)
apps/discord-bot/tsconfig.json (3)
3-3
: Consider enabling incremental compilation for faster buildsSetting
"incremental": true
can significantly improve build times in development by reusing results from previous compilations.- "incremental": false, + "incremental": true,
21-21
: Enable unused parameters checkingHaving
noUnusedParameters
disabled can lead to confusing code with unused parameters. Consider enabling it to maintain cleaner function signatures.- "noUnusedParameters": false, + "noUnusedParameters": true,
1-33
: Add source map support for better debuggingConsider adding source map generation for better debugging experience, especially useful when running the Discord bot in development.
Add the following option to your compiler options:
"compilerOptions": { + "sourceMap": true, // ... other options }
apps/discord-bot/src/structures/commandOptions/ownerOnly.ts (3)
6-8
: Improve readability of the initial command checkThe initial check is correct but could be more explicit about its intent.
- if (!command.ownerOnly || typeof command.ownerOnly !== 'boolean') + if (!command.ownerOnly || typeof command.ownerOnly !== 'boolean') { + // Command is not restricted to owners return true + }
18-30
: Add error handling for embed creationThe embed creation could fail if user data is inaccessible.
- message.channel.send({ + try { + await message.channel.send({ embeds: [ new EmbedBuilder() .setColor('DarkRed') .setTimestamp() .setAuthor({ name: message.member?.user.globalName ?? message.member?.user.username ?? '', iconURL: message.member?.user.displayAvatarURL(), }) .setThumbnail(client.user.displayAvatarURL()) .setDescription('The command you tried to run is __restricted__ for the developers of this bot and thus the command failed to execute.'), ], - }) + }) + } catch (error) { + console.error('Failed to send owner-only error message:', error) + }
31-33
: Remove unnecessary semicolon and fix code structureThe trailing semicolon is unnecessary, and the code structure can be improved.
- return false - }; + return false }🧰 Tools
🪛 Biome (1.9.4)
[error] 32-32: This code is unreachable
... because either this statement, ...
... this statement, ...
... this statement, ...
... this statement, ...
... or this statement will return from the function beforehand
(lint/correctness/noUnreachable)
apps/discord-bot/README.md (1)
82-86
: Improve clarity in technical notes-Guild commands may take time to refresh if there are a large number of different guild commands. +Guild commands may take time to refresh if there are many different guild commands.🧰 Tools
🪛 LanguageTool
[style] ~86-~86: Specify a number, remove phrase, or simply use “many” or “numerous”
Context: ...s may take time to refresh if there are a large number of different guild commands. - Collections...(LARGE_NUMBER_OF)
apps/discord-bot/src/structures/commandOptions/onlyUsers.ts (3)
5-8
: Consider adding explicit type guard for onlyUsers array.While the current check works, it could be more explicit about the expected type.
- if (!command.onlyUsers || !Array.isArray(command.onlyUsers)) + if (!Array.isArray(command.onlyUsers) || command.onlyUsers.length === 0)
18-31
: Enhance error message and add null checks.A few suggestions to improve the error handling:
- Add null check for client.user
- Provide more specific error message
- Remove unnecessary semicolon
message.channel.send({ embeds: [new EmbedBuilder() .setColor('DarkRed') .setTimestamp() .setAuthor({ name: message.member?.user.globalName ?? message.member?.user.username ?? '', iconURL: message.member?.user.displayAvatarURL(), }) - .setThumbnail(client.user.displayAvatarURL()) + .setThumbnail(client.user?.displayAvatarURL()) .setDescription('The command you tried to execute couldn\'t be ran as you are not one of the authorized users.'), ], }) - return false - }; + return false;Also consider enhancing the error message to be more specific:
- .setDescription('The command you tried to execute couldn\'t be ran as you are not one of the authorized users.'), + .setDescription(`You don't have permission to use the \`${command.name}\` command. This command is restricted to specific users.`),🧰 Tools
🪛 Biome (1.9.4)
[error] 31-31: This code is unreachable
... because either this statement, ...
... this statement, ...
... this statement, ...
... this statement, ...
... or this statement will return from the function beforehand
(lint/correctness/noUnreachable)
1-32
: Consider enhancing the permission system architecture.A few architectural suggestions:
- Consider integrating with a broader permission system that can handle role-based access control (RBAC) alongside user-specific permissions
- Add validation for user ID format to prevent invalid IDs in the
onlyUsers
array- Consider adding an interface for command options to better document the expected structure of
onlyUsers
,returnErrors
, andreturnOnlyUsersError
Would you like me to provide an example implementation of these architectural improvements?
🧰 Tools
🪛 Biome (1.9.4)
[error] 31-31: This code is unreachable
... because either this statement, ...
... this statement, ...
... this statement, ...
... this statement, ...
... or this statement will return from the function beforehand
(lint/correctness/noUnreachable)
apps/discord-bot/src/structures/commandOptions/onlyRoles.ts (1)
9-12
: Optimize the role checking logic and remove unnecessary else block.The role checking logic is correct but can be simplified by removing the unnecessary else block.
if (command.onlyRoles.some((roleID: string) => message.member?.roles.cache.has(roleID))) { return true } - else {
apps/discord-bot/src/events/ready.ts (2)
8-15
: Add error handling for activity setup.While the activity setup is correct, it should handle potential errors when setting the activity status.
- client.user?.setActivity('Humans.', { - type: ActivityType.Watching, - }) + try { + await client.user?.setActivity('Humans.', { + type: ActivityType.Watching, + }) + } catch (error) { + console.error('Failed to set activity status:', error) + }
8-61
: Consider architectural improvements for better maintainability.
- Extract command loading logic into a separate service class for better separation of concerns and testability.
- Consider implementing dependency injection for the client and configuration.
- Add interfaces for command loaders to make them pluggable and easier to test.
Example service structure:
interface CommandLoader { loadCommands(): Promise<string[]>; } class SlashCommandLoader implements CommandLoader { constructor(private rootPath: string) {} async loadCommands(): Promise<string[]> { // Implementation } } class ReadyEventHandler { constructor( private client: DiscordClient, private slashCommandLoader: CommandLoader, private contextMenuLoader: CommandLoader ) {} async handle(): Promise<void> { // Implementation } }apps/discord-bot/template-server/src/register.ts (1)
3-6
: Documentation needs enhancementWhile the documentation explains the purpose, it should also mention:
- The expected format of the metadata
- The significance of the metadata types
- The impact of re-running this registration
/** * Register the metadata to be stored by Discord. This should be a one time action. * Note: uses a Bot token for authentication, not a user token. + * + * @see https://discord.com/developers/docs/resources/application-role-connection-metadata + * + * @remarks + * - This updates the metadata schema for all users + * - Re-running will override existing metadata configuration + * - Types: number_lt(1), number_gt(2), number_eq(3), number_neq(4), + * datetime_lt(5), datetime_gt(6), boolean_eq(7), boolean_neq(8) */apps/discord-bot/template-server/README.md (1)
117-117
: Fix markdown formatting issuesSeveral markdown formatting issues need to be addressed:
- Use proper markdown link syntax for bare URLs
- Add language identifiers to code blocks
- Remove
$
from command examples-Browse https://github.com/JustinBeckwith/fitbit-discord-bot/ for a more in-depth example +Browse [fitbit-discord-bot](https://github.com/JustinBeckwith/fitbit-discord-bot/) for a more in-depth exampleAlso update code blocks to include language identifiers:
-``` +```bash $ node server.js<details> <summary>🧰 Tools</summary> <details> <summary>🪛 Markdownlint (0.37.0)</summary> 117-117: null Bare URL used (MD034, no-bare-urls) </details> </details> </blockquote></details> <details> <summary>apps/discord-bot/src/structures/managers/modalForms.ts (1)</summary><blockquote> `5-5`: **Add JSDoc documentation for the ModalManager function** Consider adding comprehensive JSDoc documentation to describe the function's purpose, parameters, and usage. ```diff +/** + * Initializes and loads modal form handlers for the Discord bot + * @param client - The Discord client instance to register modal forms with + * @param rootPath - The root directory path where modal form files are located + * @throws {Error} When modal form files cannot be loaded or are invalid + */ export async function ModalManager(client: DiscordClient, rootPath: string): Promise<void> {
apps/discord-bot/src/structures/commandOptions/anyClientPermissions.ts (1)
1-31
: Consider extracting shared permission checking logicThis file shares significant code with
anyUserPermissions.ts
. Consider extracting common functionality into a shared utility.Create a new utility file
permissionUtils.ts
:import { DiscordClient, Interaction, Message, EmbedBuilder } from 'discord.js' import { AnyCommand } from '../../types.js' export function createPermissionErrorEmbed( client: DiscordClient, message: Message | Interaction<'cached'>, permissions: bigint[], errorMessage: string ): EmbedBuilder { return new EmbedBuilder() .setColor('DarkRed') .setTimestamp() .setAuthor({ name: message.member?.user.globalName ?? message.member?.user.username ?? '', iconURL: message.member?.user.displayAvatarURL(), }) .setThumbnail(client.user.displayAvatarURL()) .setDescription(errorMessage) }This would reduce code duplication and make maintenance easier.
apps/discord-bot/src/structures/commandOptions/allUserPermissions.ts (2)
8-8
: Simplify permission check with optional chainingThe permission check can be simplified while maintaining type safety.
- const missingPermissions: Array<PermissionsString> | undefined = message.member?.permissions.missing(command.allUserPermissions) + const missingPermissions = message.member?.permissions?.missing(command.allUserPermissions)
24-27
: Enhance user identification robustnessThe user identification logic could be more robust by adding a fallback for the display name.
.setAuthor({ - name: message.member?.user.globalName ?? message.member?.user.username ?? '', + name: message.member?.user.globalName ?? message.member?.user.username ?? message.member?.user.tag ?? 'Unknown User', iconURL: message.member?.user.displayAvatarURL(), })apps/discord-bot/src/events/interactionCreate.ts (1)
66-66
: Remove unnecessary semicolonThe semicolon after the closing brace of the modal submit block is unnecessary.
}; - }; + }apps/discord-bot/src/types.ts (5)
5-17
: Enhance module augmentation documentationThe comment could be more descriptive about the purpose of extending the Client interface.
Consider replacing the existing comment with:
-}; // Extends Discord.js Client to allow for the Map caches. +}; // Extends Discord.js Client to add command registries and cooldown management capabilities
19-47
: Enhance type safety and organization of CommandOptionsThe interface could benefit from the following improvements:
- Use more specific types for permissions
- Group related properties using nested interfaces
Consider applying these changes:
import { PermissionsBitField } from 'discord.js'; interface CooldownOptions { channelCooldown?: number; globalCooldown?: number; guildCooldown?: number; } interface PermissionOptions { allClientPermissions?: Array<keyof typeof PermissionsBitField.Flags>; allUserPermissions?: Array<keyof typeof PermissionsBitField.Flags>; anyClientPermissions?: Array<keyof typeof PermissionsBitField.Flags>; anyUserPermissions?: Array<keyof typeof PermissionsBitField.Flags>; } interface TargetingOptions { allowInDms?: boolean; onlyChannels?: Array<string>; onlyGuilds?: Array<string>; onlyRoles?: Array<string>; onlyUsers?: Array<string>; ownerOnly?: boolean; } export interface CommandOptions extends CooldownOptions, PermissionOptions, TargetingOptions { // Error handling options remain the same returnErrors?: boolean; // ... }
49-114
: Add JSDoc documentation for command interfacesThe command interfaces would benefit from detailed documentation explaining their purpose and usage.
Consider adding JSDoc comments like this:
/** * Represents a message-based command that is triggered by a prefix. * @example * const pingCommand: MessageCommand = { * name: 'ping', * run: (client, message) => { * message.reply('Pong!'); * } * }; */ export interface MessageCommand extends CommandOptions { // ... existing properties }
115-116
: Derive InteractionTypeOptions from AnyCommand typeTo maintain consistency and reduce potential errors, consider deriving the interaction type options from the AnyCommand type.
Apply this change:
export type AnyCommand = MessageCommand | ButtonCommand | SelectMenu | ModalForm | SlashCommand | ContextMenu; // Derive from AnyCommand to ensure consistency export type InteractionTypeOptions = { [K in AnyCommand as K extends any ? K['constructor']['name'] : never]: K; }[keyof AnyCommand];
1-116
: Well-structured type system for Discord botThe overall architecture provides a robust foundation for type-safe Discord bot development. The interfaces are well-designed and extensible, allowing for easy addition of new command types in the future.
Consider creating a separate documentation file (e.g.,
ARCHITECTURE.md
) explaining the type system and providing examples of how to implement each command type.apps/discord-bot/src/bot.ts (1)
5-5
: Consider using a more robust database for cooldown managementUsing
simple-json-db
for cooldown management might lead to scalability issues in production. Consider using Redis or a similar in-memory database for better performance and reliability with cooldowns.Also applies to: 35-35
apps/discord-bot/src/structures/managers/slashCommands.ts (1)
11-27
: Add JSDoc documentation for interfacesThe interfaces are well-structured but lack documentation. Consider adding JSDoc comments to improve maintainability:
+/** + * Represents a global Discord application command + * @interface GlobalCommandArray + */ interface GlobalCommandArray { name: string nsfw?: boolean type: ApplicationCommandType description?: string options?: Array<SlashCommandOptions> }; +/** + * Represents guild-specific Discord application commands + * @interface GuildCommandObjects + */ interface GuildCommandObjects { [key: string]: Array<{ name: string nsfw?: boolean description?: string type: ApplicationCommandType options?: Array<SlashCommandOptions> }> };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
apps/discord-bot/template-server/package-lock.json
is excluded by!**/package-lock.json
📒 Files selected for processing (53)
apps/discord-bot/.gitignore
(1 hunks)apps/discord-bot/LICENSE
(1 hunks)apps/discord-bot/README.md
(1 hunks)apps/discord-bot/nodemon.json
(1 hunks)apps/discord-bot/package.json
(1 hunks)apps/discord-bot/src/bot.ts
(1 hunks)apps/discord-bot/src/config.ts
(1 hunks)apps/discord-bot/src/events/errorManager.ts
(1 hunks)apps/discord-bot/src/events/interactionCreate.ts
(1 hunks)apps/discord-bot/src/events/messageCreate.ts
(1 hunks)apps/discord-bot/src/events/ready.ts
(1 hunks)apps/discord-bot/src/interactions/buttons/deleteOutput.ts
(1 hunks)apps/discord-bot/src/interactions/contextMenus/getUsername.ts
(1 hunks)apps/discord-bot/src/interactions/modalForms/exampleModal.ts
(1 hunks)apps/discord-bot/src/interactions/selectMenus/selectMenuExample.ts
(1 hunks)apps/discord-bot/src/interactions/slashCommands/exampleModal.ts
(1 hunks)apps/discord-bot/src/interactions/slashCommands/ping.ts
(1 hunks)apps/discord-bot/src/messageCommands/callSelectMenu.ts
(1 hunks)apps/discord-bot/src/messageCommands/eval.ts
(1 hunks)apps/discord-bot/src/messageCommands/ping.ts
(1 hunks)apps/discord-bot/src/structures/commandOptions/allClientPermissions.ts
(1 hunks)apps/discord-bot/src/structures/commandOptions/allUserPermissions.ts
(1 hunks)apps/discord-bot/src/structures/commandOptions/anyClientPermissions.ts
(1 hunks)apps/discord-bot/src/structures/commandOptions/anyUserPermissions.ts
(1 hunks)apps/discord-bot/src/structures/commandOptions/channelCooldown.ts
(1 hunks)apps/discord-bot/src/structures/commandOptions/globalCooldown.ts
(1 hunks)apps/discord-bot/src/structures/commandOptions/guildCooldown.ts
(1 hunks)apps/discord-bot/src/structures/commandOptions/onlyChannels.ts
(1 hunks)apps/discord-bot/src/structures/commandOptions/onlyGuilds.ts
(1 hunks)apps/discord-bot/src/structures/commandOptions/onlyRoles.ts
(1 hunks)apps/discord-bot/src/structures/commandOptions/onlyUsers.ts
(1 hunks)apps/discord-bot/src/structures/commandOptions/ownerOnly.ts
(1 hunks)apps/discord-bot/src/structures/commandOptions/processor.ts
(1 hunks)apps/discord-bot/src/structures/managers/buttonCommands.ts
(1 hunks)apps/discord-bot/src/structures/managers/events.ts
(1 hunks)apps/discord-bot/src/structures/managers/messageCommands.ts
(1 hunks)apps/discord-bot/src/structures/managers/modalForms.ts
(1 hunks)apps/discord-bot/src/structures/managers/selectMenus.ts
(1 hunks)apps/discord-bot/src/structures/managers/slashCommands.ts
(1 hunks)apps/discord-bot/src/types.ts
(1 hunks)apps/discord-bot/src/utils/fileReader.ts
(1 hunks)apps/discord-bot/template-server/.gitignore
(1 hunks)apps/discord-bot/template-server/README.md
(1 hunks)apps/discord-bot/template-server/package.json
(1 hunks)apps/discord-bot/template-server/src/config.ts
(1 hunks)apps/discord-bot/template-server/src/discord.ts
(1 hunks)apps/discord-bot/template-server/src/register.ts
(1 hunks)apps/discord-bot/template-server/src/server.ts
(1 hunks)apps/discord-bot/template-server/src/storage.ts
(1 hunks)apps/discord-bot/tsconfig.json
(1 hunks)libs/external/aceternity/tailwind.config.ts
(1 hunks)libs/website/feature/faq/index.ts
(1 hunks)libs/website/feature/stats/index.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (26)
- apps/discord-bot/template-server/.gitignore
- apps/discord-bot/src/interactions/modalForms/exampleModal.ts
- apps/discord-bot/LICENSE
- apps/discord-bot/.gitignore
- libs/website/feature/faq/index.ts
- apps/discord-bot/src/interactions/selectMenus/selectMenuExample.ts
- apps/discord-bot/src/interactions/slashCommands/ping.ts
- apps/discord-bot/template-server/src/config.ts
- apps/discord-bot/src/interactions/buttons/deleteOutput.ts
- apps/discord-bot/src/interactions/contextMenus/getUsername.ts
- libs/website/feature/stats/index.ts
- libs/external/aceternity/tailwind.config.ts
- apps/discord-bot/src/config.ts
- apps/discord-bot/src/interactions/slashCommands/exampleModal.ts
- apps/discord-bot/nodemon.json
- apps/discord-bot/src/utils/fileReader.ts
- apps/discord-bot/src/messageCommands/callSelectMenu.ts
- apps/discord-bot/src/structures/managers/selectMenus.ts
- apps/discord-bot/template-server/package.json
- apps/discord-bot/src/structures/commandOptions/processor.ts
- apps/discord-bot/src/messageCommands/ping.ts
- apps/discord-bot/template-server/src/server.ts
- apps/discord-bot/src/structures/managers/messageCommands.ts
- apps/discord-bot/src/messageCommands/eval.ts
- apps/discord-bot/package.json
- apps/discord-bot/template-server/src/discord.ts
🧰 Additional context used
🪛 Biome (1.9.4)
apps/discord-bot/src/structures/commandOptions/onlyRoles.ts
[error] 31-32: This code is unreachable
... because either this statement, ...
... this statement, ...
... this statement, ...
... this statement, ...
... or this statement will return from the function beforehand
(lint/correctness/noUnreachable)
apps/discord-bot/src/structures/commandOptions/allClientPermissions.ts
[error] 33-34: This code is unreachable
... because either this statement, ...
... this statement, ...
... this statement, ...
... this statement, ...
... or this statement will return from the function beforehand
(lint/correctness/noUnreachable)
apps/discord-bot/src/structures/commandOptions/onlyGuilds.ts
[error] 31-32: This code is unreachable
... because either this statement, ...
... this statement, ...
... this statement, ...
... this statement, ...
... or this statement will return from the function beforehand
(lint/correctness/noUnreachable)
apps/discord-bot/src/structures/commandOptions/ownerOnly.ts
[error] 32-32: This code is unreachable
... because either this statement, ...
... this statement, ...
... this statement, ...
... this statement, ...
... or this statement will return from the function beforehand
(lint/correctness/noUnreachable)
apps/discord-bot/src/structures/commandOptions/allUserPermissions.ts
[error] 33-34: This code is unreachable
... because either this statement, ...
... this statement, ...
... this statement, ...
... this statement, ...
... or this statement will return from the function beforehand
(lint/correctness/noUnreachable)
apps/discord-bot/src/structures/commandOptions/onlyChannels.ts
[error] 31-32: This code is unreachable
... because either this statement, ...
... this statement, ...
... this statement, ...
... this statement, ...
... or this statement will return from the function beforehand
(lint/correctness/noUnreachable)
apps/discord-bot/src/structures/commandOptions/onlyUsers.ts
[error] 31-31: This code is unreachable
... because either this statement, ...
... this statement, ...
... this statement, ...
... this statement, ...
... or this statement will return from the function beforehand
(lint/correctness/noUnreachable)
🪛 LanguageTool
apps/discord-bot/template-server/README.md
[style] ~8-~8: Consider removing “of” to be more concise
Context: ...ole-discord-bot) ## Project structure All of the files for the project are on the left-h...
(ALL_OF_THE)
apps/discord-bot/README.md
[uncategorized] ~19-~19: Did you mean “I”?
Context: ...ple-json-dbfor the cooldown system as i rage quit and can't do it with
fs` mys...
(I_LOWERCASE_PREMIUM)
[misspelling] ~19-~19: This word is normally spelled with a hyphen.
Context: ...e-json-dbfor the cooldown system as i rage quit and can't do it with
fs` myself. - Fix...
(EN_COMPOUNDS_RAGE_QUIT)
[style] ~78-~78: The phrase “a variety of” may be wordy. To make your writing clearer, consider replacing it.
Context: ...text menus, and modal forms. - Includes a variety of commonly used command options (not appl...
(A_VARIETY_OF)
[style] ~86-~86: Specify a number, remove phrase, or simply use “many” or “numerous”
Context: ...s may take time to refresh if there are a large number of different guild commands. - Collections...
(LARGE_NUMBER_OF)
🪛 Markdownlint (0.37.0)
apps/discord-bot/template-server/README.md
74-74: null
Dollar signs used before commands without showing output
(MD014, commands-show-output)
80-80: null
Dollar signs used before commands without showing output
(MD014, commands-show-output)
90-90: null
Dollar signs used before commands without showing output
(MD014, commands-show-output)
117-117: null
Bare URL used
(MD034, no-bare-urls)
11-11: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
35-35: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
41-41: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
50-50: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
60-60: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
73-73: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
79-79: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
89-89: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
95-95: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
apps/discord-bot/README.md
2-2: null
Images should have alternate text (alt text)
(MD045, no-alt-text)
3-3: null
Images should have alternate text (alt text)
(MD045, no-alt-text)
4-4: null
Images should have alternate text (alt text)
(MD045, no-alt-text)
5-5: null
Images should have alternate text (alt text)
(MD045, no-alt-text)
6-6: null
Images should have alternate text (alt text)
(MD045, no-alt-text)
7-7: null
Images should have alternate text (alt text)
(MD045, no-alt-text)
🔇 Additional comments (37)
apps/discord-bot/template-server/src/storage.ts (1)
3-5
: Previous feedback about type safety and error handling still applies
The implementation needs the improvements suggested in the previous review regarding:
- Replacing
any
with a proper interface - Adding error handling
- Removing unnecessary
async/await
- Adding input validation
apps/discord-bot/src/structures/commandOptions/guildCooldown.ts (3)
9-9
: Sanitize database key to prevent potential injection issues.
This code concatenates user-controlled inputs (guild ID, interaction type, command name, member ID) into the DB key without sanitizing or limiting special characters (e.g., dots). Consider replacing dots or other special characters in user input to avoid key collisions or injection vulnerabilities.
13-14
: Add error handling on cooldown DB write.
Code silently fails if the database write operation throws an error, which may lead to inconsistent cooldown states. Wrap the DB access in a try/catch block and log or handle errors gracefully.
5-8
: Consider adding test coverage and verifying time precision.
- Test Coverage: These lines handle guild-specific cooldown logic and user notification, but they have no test coverage. You could create unit tests to validate command eligibility, cooldown expiration, and error message logic.
- Time Precision: Using
Math.floor
for time difference is fine, but confirm that small differences in milliseconds are acceptable for your application. If higher precision is required, you might store or compare times at full precision.
Also applies to: 23-35
apps/discord-bot/src/structures/commandOptions/globalCooldown.ts (3)
11-11
: Check for uninitialized database.
Accessing client.cooldownDB?.get
is safe in principle, but you should confirm that cooldownDB
is always defined before usage. If it's undefined
, the cooldown data might never be stored, causing unintended command execution behavior.
9-9
: Sanitize the DB key and add error handling.
- DB Key Construction: The concatenation of user inputs into the DB key may lead to collisions or injection if certain characters (e.g., dots) are used.
- Error Handling: Wrap the DB write in try/catch so the application can log or recover from database errors.
Also applies to: 13-14
5-8
: Add tests for global cooldown flow.
Ensure coverage for:
- Normal vs. expired cooldown scenarios.
- Missing or invalid cooldown values.
- Error/notification behavior.
This helps verify logic correctness and user-facing cooldown messages.
Also applies to: 23-35
apps/discord-bot/src/structures/commandOptions/channelCooldown.ts (3)
9-9
: Fix the typo in the database key.
There is a spelling error "channelCoolown" instead of "channelCooldown", which can break cooldown tracking.
- const dbData = `channelCoolown.${message.channel?.id}.${interactionType}.${command.name}.${message.member?.id}`
+ const dbData = `channelCooldown.${message.channel?.id}.${interactionType}.${command.name}.${message.member?.id}`
5-8
: Extract common cooldown logic to a shared utility.
The logic here is nearly identical to the guild/global cooldown approach. Refactor to reduce code duplication by creating a reusable helper function to handle cooldown checks and notification for all scopes (channel, guild, global).
Also applies to: 13-14
23-35
: Add channel-specific cooldown tests.
Consider testing:
- Whether the channel cooldown is enforced as expected in different channels or guild contexts.
- How errors or messages are displayed when a cooldown is active.
apps/discord-bot/src/structures/commandOptions/onlyChannels.ts (4)
1-4
: LGTM! Clean and well-organized imports
The imports are properly organized with type imports separated from regular imports.
9-17
: Simplify control flow structure
The nested if-else blocks make the code harder to follow. Consider flattening the structure as suggested in the previous review.
31-31
: Remove unnecessary semicolon causing unreachable code
The semicolon after the closing brace is unnecessary and causes a linting error.
🧰 Tools
🪛 Biome (1.9.4)
[error] 31-32: This code is unreachable
... because either this statement, ...
... this statement, ...
... this statement, ...
... this statement, ...
... or this statement will return from the function beforehand
(lint/correctness/noUnreachable)
5-32
: Add comprehensive tests for permission checks
This function handles critical channel access control. Comprehensive testing is essential as mentioned in the previous review.
🧰 Tools
🪛 Biome (1.9.4)
[error] 31-32: This code is unreachable
... because either this statement, ...
... this statement, ...
... this statement, ...
... this statement, ...
... or this statement will return from the function beforehand
(lint/correctness/noUnreachable)
apps/discord-bot/src/structures/commandOptions/onlyGuilds.ts (1)
1-4
: LGTM! Clean import organization
The imports are well-organized, separating type imports from value imports.
apps/discord-bot/tsconfig.json (3)
4-8
: LGTM: Basic compilation settings are well configured
The basic compilation settings are appropriate for a modern Node.js application:
- Using
ESNext
target allows for modern JavaScript features Node16
module system is suitable for current LTS versions- JSON module resolution is correctly enabled
9-13
: LGTM: Strong type checking configuration
The strict type checking options are well configured to catch potential issues early in development:
- Strict mode and its related flags are enabled
- No implicit
any
types - Proper handling of
this
context - Override checking enabled
Also applies to: 15-17, 19-20, 22-22
23-31
: LGTM: Output and compatibility settings are appropriate
The output and compatibility settings are well configured:
- Declaration files generation is enabled with source maps
- Proper error handling with
noEmitOnError
- Good module interop settings
- Consistent file casing enforced
apps/discord-bot/src/structures/commandOptions/ownerOnly.ts (2)
1-5
: LGTM! Imports are well-organized and complete.
All necessary types and utilities are properly imported.
9-11
: Improve type safety in user ID extraction
The nested nullish coalescing and type casting could be simplified and made more type-safe.
apps/discord-bot/src/structures/commandOptions/onlyUsers.ts (1)
1-4
: LGTM! Imports are well-organized.
The imports are properly typed and logically ordered, with type imports preceding concrete imports.
apps/discord-bot/src/structures/commandOptions/onlyRoles.ts (4)
1-4
: LGTM! Imports are well-organized and properly typed.
5-8
: LGTM! Well-structured function signature with proper type guards.
The early returns effectively handle edge cases and null checks.
27-27
: Fix incorrect role mention format
The role mention format should use @&
instead of #
.
- .setDescription(`The command you tried to execute couldn't be executed as you are missing one of these required roles:\n${command.onlyRoles.map((roleID: string) => `↳ <#${roleID}>`).join('\n')}`),
+ .setDescription(`The command you tried to execute couldn't be executed as you are missing one of these required roles:\n${command.onlyRoles.map((roleID: string) => `↳ <@&${roleID}>`).join('\n')}`),
5-31
: Consider enhancing role validation and error message customization
A few architectural improvements to consider:
- Add role resolution validation to ensure the specified role IDs exist in the guild
- Allow customization of error messages through command options
- Consider implementing role hierarchies or role combinations for more complex permission scenarios
Let's verify if the roles are properly validated before use:
apps/discord-bot/src/events/ready.ts (3)
1-7
: LGTM! Clean and well-organized imports.
The imports are properly organized with type imports separated, and all necessary dependencies are included.
16-23
: Previous review comments about slash commands loading still apply.
The implementation still has type safety and error handling issues that were identified in the previous review.
25-32
: Previous review comments about context menus loading still apply.
The implementation has the same issues as the slash commands loading that were identified in the previous review.
apps/discord-bot/template-server/src/register.ts (1)
32-54
: LGTM: Error handling implementation
The error handling is well-implemented with:
- Proper HTTP response status checking
- Detailed error messages
- Error propagation with context
apps/discord-bot/template-server/README.md (3)
11-22
: Update project structure to reflect TypeScript implementation
The project structure shows .js
files, but this is a TypeScript project.
🧰 Tools
🪛 Markdownlint (0.37.0)
11-11: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
73-75
: Add TypeScript-specific running instructions
Since this is a TypeScript project, please add compilation and development mode instructions.
🧰 Tools
🪛 Markdownlint (0.37.0)
74-74: null
Dollar signs used before commands without showing output
(MD014, commands-show-output)
73-73: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
7-7
: Add testing documentation section
As mentioned in the PR objectives, testing documentation is required.
apps/discord-bot/src/structures/managers/modalForms.ts (1)
1-3
: LGTM! Well-structured imports
The imports are properly typed and follow a logical organization pattern.
apps/discord-bot/src/structures/commandOptions/allUserPermissions.ts (2)
19-20
: Remove redundant optional chaining
The optional chaining on message?.channel
is unnecessary since the channel existence is already checked in the if condition above.
- message?.channel.send({
+ message.channel.send({
33-34
: Remove unreachable semicolon
The semicolon after the closing brace is unreachable and should be removed.
return false
- };
+ }
🧰 Tools
🪛 Biome (1.9.4)
[error] 33-34: This code is unreachable
... because either this statement, ...
... this statement, ...
... this statement, ...
... this statement, ...
... or this statement will return from the function beforehand
(lint/correctness/noUnreachable)
apps/discord-bot/src/structures/commandOptions/allClientPermissions.ts (1)
8-8
: Simplify guild member access chain
The optional chaining in the guild members access is redundant since we already check for guild existence.
- const missingPermissions: Array<PermissionsString> | undefined = message.guild?.members?.me?.permissions.missing(command.allClientPermissions)
+ const missingPermissions: Array<PermissionsString> | undefined = message.guild.members.me?.permissions.missing(command.allClientPermissions)
apps/discord-bot/src/bot.ts (1)
37-44
: LGTM! Well-structured command storage
Good use of TypeScript Maps for different command types, maintaining type safety and clear separation of concerns.
export async function getDiscordTokens(userId: string) { | ||
return store.get(`discord-${userId}`) | ||
} |
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.
Add token expiration checks
The function should verify token expiration before returning stored tokens. This prevents usage of expired tokens and ensures timely refresh.
Add expiration checks:
interface DiscordTokens {
access_token: string;
refresh_token: string;
expires_at: number;
}
export async function getDiscordTokens(userId: string): Promise<DiscordTokens | null> {
const tokens = store.get(`discord-${userId}`) as DiscordTokens | null;
if (!tokens) return null;
// Check if token is expired (with 5 minute buffer)
if (Date.now() >= tokens.expires_at - 300000) {
// Token is expired or expiring soon
// TODO: Implement token refresh logic
return null;
}
return tokens;
}
"noFallthroughCasesInSwitch": true, | ||
"noImplicitAny": true, | ||
"noImplicitOverride": true, | ||
"noImplicitReturns": false, |
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.
🛠️ Refactor suggestion
Enable implicit returns checking for better type safety
Having noImplicitReturns
disabled means functions might have code paths that don't return a value, potentially leading to runtime errors.
- "noImplicitReturns": false,
+ "noImplicitReturns": true,
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
"noImplicitReturns": false, | |
"noImplicitReturns": true, |
"strictNullChecks": true, | ||
"strictPropertyInitialization": true, | ||
"allowUnreachableCode": false, | ||
"allowUnusedLabels": true, |
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.
🛠️ Refactor suggestion
Disable unused labels for cleaner code
Having allowUnusedLabels
enabled can lead to messy code with redundant labels. Consider disabling it to enforce cleaner code practices.
- "allowUnusedLabels": true,
+ "allowUnusedLabels": false,
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
"allowUnusedLabels": true, | |
"allowUnusedLabels": false, |
else { | ||
if (command.returnErrors === false || command.returnOwnerOnlyError === false) | ||
return false | ||
if (!message.channel || message.channel.type !== ChannelType.GuildText) | ||
return false | ||
|
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.
🛠️ Refactor suggestion
Simplify error handling logic
The nested conditions can be flattened to improve readability and maintainability.
- else {
- if (command.returnErrors === false || command.returnOwnerOnlyError === false)
- return false
- if (!message.channel || message.channel.type !== ChannelType.GuildText)
- return false
+ if (command.returnErrors === false || command.returnOwnerOnlyError === false) {
+ return false
+ }
+ if (!message.channel || message.channel.type !== ChannelType.GuildText) {
+ return false
+ }
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
else { | |
if (command.returnErrors === false || command.returnOwnerOnlyError === false) | |
return false | |
if (!message.channel || message.channel.type !== ChannelType.GuildText) | |
return false | |
if (command.returnErrors === false || command.returnOwnerOnlyError === false) { | |
return false | |
} | |
if (!message.channel || message.channel.type !== ChannelType.GuildText) { | |
return false | |
} |
<a href="https://discord.gg/VStdRr8nP2"><img src="https://img.shields.io/badge/discord-invite-5865f2?style=for-the-badge&logo=discord&logoColor=white"></a> | ||
<img src="https://img.shields.io/github/issues/RileCraft/DiscordBot-Template-ts.svg?style=for-the-badge"> | ||
<img src="https://img.shields.io/github/forks/RileCraft/DiscordBot-Template-ts.svg?style=for-the-badge"> | ||
<img src="https://img.shields.io/github/stars/RileCraft/DiscordBot-Template-ts.svg?style=for-the-badge"> |
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.
🛠️ Refactor suggestion
Update repository links for this project
The badge links still point to the original template repository (RileCraft/DiscordBot-Template-ts
). These should be updated to point to this project's repository.
🧰 Tools
🪛 Markdownlint (0.37.0)
4-4: null
Images should have alternate text (alt text)
(MD045, no-alt-text)
5-5: null
Images should have alternate text (alt text)
(MD045, no-alt-text)
6-6: null
Images should have alternate text (alt text)
(MD045, no-alt-text)
7-7: null
Images should have alternate text (alt text)
(MD045, no-alt-text)
intents: [ | ||
GatewayIntentBits.Guilds, | ||
GatewayIntentBits.GuildMessages, | ||
GatewayIntentBits.GuildPresences, | ||
GatewayIntentBits.DirectMessages, | ||
GatewayIntentBits.MessageContent, // Only for bots with message content intent access. | ||
GatewayIntentBits.DirectMessageReactions, | ||
GatewayIntentBits.GuildMembers, | ||
GatewayIntentBits.GuildMessageReactions, | ||
GatewayIntentBits.GuildWebhooks, | ||
GatewayIntentBits.GuildVoiceStates, | ||
GatewayIntentBits.GuildInvites, | ||
], |
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.
🛠️ Refactor suggestion
Optimize Discord intents for security
The bot is requesting more intents than might be necessary, including the privileged MessageContent
intent. Consider:
- Removing unnecessary intents to follow the principle of least privilege
- Documenting why each intent is needed
- Ensuring the bot has approval for privileged intents
intents: [
GatewayIntentBits.Guilds,
- GatewayIntentBits.GuildMessages,
- GatewayIntentBits.GuildPresences,
- GatewayIntentBits.DirectMessages,
- GatewayIntentBits.MessageContent, // Only for bots with message content intent access.
- GatewayIntentBits.DirectMessageReactions,
GatewayIntentBits.GuildMembers,
- GatewayIntentBits.GuildMessageReactions,
- GatewayIntentBits.GuildWebhooks,
GatewayIntentBits.GuildVoiceStates,
- GatewayIntentBits.GuildInvites,
],
Committable suggestion skipped: line range outside the PR's diff.
await MessageCMDManager(client, __dirname) | ||
await EventManager(client, __dirname) | ||
await ButtonManager(client, __dirname) | ||
await SelectMenuManager(client, __dirname) | ||
await ModalManager(client, __dirname) | ||
await client.login(BOT_TOKEN) | ||
await SlashManager(client, __dirname) // Includes context menu handling as they belong to same command type. | ||
})() |
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.
🛠️ Refactor suggestion
Add error handling and startup logging
The initialization sequence lacks error handling and logging. Consider:
- Adding try-catch blocks for each manager initialization
- Adding startup logging for better observability
- Implementing a graceful shutdown mechanism
+ try {
await MessageCMDManager(client, __dirname)
+ console.log('Message commands loaded')
await EventManager(client, __dirname)
+ console.log('Events loaded')
await ButtonManager(client, __dirname)
+ console.log('Button commands loaded')
await SelectMenuManager(client, __dirname)
+ console.log('Select menus loaded')
await ModalManager(client, __dirname)
+ console.log('Modal forms loaded')
await client.login(BOT_TOKEN)
+ console.log('Bot logged in successfully')
await SlashManager(client, __dirname)
+ console.log('Slash commands loaded')
+ } catch (error) {
+ console.error('Failed to initialize bot:', error)
+ process.exit(1)
+ }
+
+ process.on('SIGTERM', () => {
+ console.log('Received SIGTERM, shutting down gracefully')
+ client.destroy()
+ process.exit(0)
+ })
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
await MessageCMDManager(client, __dirname) | |
await EventManager(client, __dirname) | |
await ButtonManager(client, __dirname) | |
await SelectMenuManager(client, __dirname) | |
await ModalManager(client, __dirname) | |
await client.login(BOT_TOKEN) | |
await SlashManager(client, __dirname) // Includes context menu handling as they belong to same command type. | |
})() | |
try { | |
await MessageCMDManager(client, __dirname) | |
console.log('Message commands loaded') | |
await EventManager(client, __dirname) | |
console.log('Events loaded') | |
await ButtonManager(client, __dirname) | |
console.log('Button commands loaded') | |
await SelectMenuManager(client, __dirname) | |
console.log('Select menus loaded') | |
await ModalManager(client, __dirname) | |
console.log('Modal forms loaded') | |
await client.login(BOT_TOKEN) | |
console.log('Bot logged in successfully') | |
await SlashManager(client, __dirname) // Includes context menu handling as they belong to same command type. | |
console.log('Slash commands loaded') | |
} catch (error) { | |
console.error('Failed to initialize bot:', error) | |
process.exit(1) | |
} | |
process.on('SIGTERM', () => { | |
console.log('Received SIGTERM, shutting down gracefully') | |
client.destroy() | |
process.exit(0) | |
}) | |
})() |
|
||
if (allSlashCommandsFiles.length > 0) { | ||
for (const slashCommandFile of allSlashCommandsFiles) { | ||
const slashCommand: SlashCommand | undefined = (await import(`file:///${slashCommandFile}`))?.Slash |
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.
Secure dynamic imports and add validation
The dynamic imports using file:///
protocol could pose security risks. Consider:
- Adding path validation
- Implementing a whitelist mechanism
- Adding error handling for malformed imports
- const slashCommand: SlashCommand | undefined = (await import(`file:///${slashCommandFile}`))?.Slash
+ const normalizedPath = path.normalize(slashCommandFile)
+ if (!normalizedPath.startsWith(rootPath)) {
+ console.error(`Invalid command path: ${slashCommandFile}`)
+ continue
+ }
+ try {
+ const slashCommand: SlashCommand | undefined = (await import(normalizedPath))?.Slash
+ } catch (error) {
+ console.error(`Failed to import command from ${slashCommandFile}:`, error)
+ continue
+ }
Also applies to: 70-70
export async function SlashManager(client: DiscordClient, rootPath: string): Promise<void> { | ||
const allSlashCommandsFiles = fileReader(`${rootPath}/interactions/slashCommands`) | ||
const allContextMenusFiles = fileReader(`${rootPath}/interactions/contextMenus`) | ||
const rest: REST = new REST({ version: '10' }).setToken(client.token) |
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.
🛠️ Refactor suggestion
Secure the client token handling
The client token is directly used in the REST initialization. Consider:
- Adding token validation
- Implementing token rotation capability
- Adding error handling for invalid tokens
- const rest: REST = new REST({ version: '10' }).setToken(client.token)
+ if (!client.token) {
+ throw new Error('Bot token is not configured')
+ }
+ const rest: REST = new REST({ version: '10' }).setToken(client.token)
Committable suggestion skipped: line range outside the PR's diff.
// try { | ||
await rest.put(Routes.applicationCommands(client.application.id), { | ||
body: globalCommandsArray, | ||
}) | ||
|
||
for (const guildObject of Object.entries(guildCommandsObject)) { | ||
await rest.put(Routes.applicationGuildCommands(client.application.id, guildObject[0]), { | ||
body: guildObject[1], | ||
}) | ||
}; | ||
// } | ||
// catch (error: unknown) { | ||
// console.log(error) | ||
// }; |
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.
Implement proper error handling and rate limiting
The command registration lacks error handling and rate limit consideration. Uncomment and improve the error handling:
- // try {
+ try {
+ console.log(`Registering ${globalCommandsArray.length} global commands`)
await rest.put(Routes.applicationCommands(client.application.id), {
body: globalCommandsArray,
})
+ console.log(`Registering guild commands for ${Object.keys(guildCommandsObject).length} guilds`)
for (const guildObject of Object.entries(guildCommandsObject)) {
+ try {
await rest.put(Routes.applicationGuildCommands(client.application.id, guildObject[0]), {
body: guildObject[1],
})
+ } catch (error) {
+ console.error(`Failed to register commands for guild ${guildObject[0]}:`, error)
+ }
};
- // }
- // catch (error: unknown) {
- // console.log(error)
- // };
+ } catch (error) {
+ console.error('Failed to register commands:', error)
+ throw error
+ }
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
// try { | |
await rest.put(Routes.applicationCommands(client.application.id), { | |
body: globalCommandsArray, | |
}) | |
for (const guildObject of Object.entries(guildCommandsObject)) { | |
await rest.put(Routes.applicationGuildCommands(client.application.id, guildObject[0]), { | |
body: guildObject[1], | |
}) | |
}; | |
// } | |
// catch (error: unknown) { | |
// console.log(error) | |
// }; | |
try { | |
console.log(`Registering ${globalCommandsArray.length} global commands`) | |
await rest.put(Routes.applicationCommands(client.application.id), { | |
body: globalCommandsArray, | |
}) | |
console.log(`Registering guild commands for ${Object.keys(guildCommandsObject).length} guilds`) | |
for (const guildObject of Object.entries(guildCommandsObject)) { | |
try { | |
await rest.put(Routes.applicationGuildCommands(client.application.id, guildObject[0]), { | |
body: guildObject[1], | |
}) | |
} catch (error) { | |
console.error(`Failed to register commands for guild ${guildObject[0]}:`, error) | |
} | |
}; | |
} catch (error) { | |
console.error('Failed to register commands:', error) | |
throw error | |
} |
Scaffolding the community server discord bot as a pre-requisite for linking server roles to 3rd party applications (beginning with portal in issue #217)
Checklist
Summary by CodeRabbit
New Features
Documentation
Chores
.gitignore
file to exclude unnecessary files from version control.