Skip to content

[Reviewed] fix: add folder existence checks (Item 3) #27

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.

Sign up for GitHub

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

Open
dromero14521 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[Reviewed] fix: add folder existence checks (Item 3) #27

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/bot.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@ loadCommands('prefixcommands', client.prefixCommands);

const loadEvents = () => {
const eventsPath = path.join(__dirname, 'events');
if (!fs.existsSync(eventsPath)) {
console.warn(`[WARNING] Events directory missing at ${eventsPath}. Auto-creating it.`);
fs.mkdirSync(eventsPath, { recursive: true });
Copy link

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fs.mkdirSync(eventsPath, { recursive: true }) can still throw (e.g., read-only filesystem, missing permissions). Since the intent is "warn and keep running", wrap the mkdir in try/catch (and consider returning early or exiting) so the bot doesn’t crash with an unhandled exception when it can’t create the directory.

Suggested change
fs.mkdirSync(eventsPath, { recursive: true });
try {
fs.mkdirSync(eventsPath, { recursive: true });
} catch (error) {
console.warn(`[WARNING] Failed to create events directory at ${eventsPath}: ${error.message}`);
return;
}

Copilot uses AI. Check for mistakes.
}
const eventFiles = fs.readdirSync(eventsPath).filter(f => f.endsWith('.js'));

for (const file of eventFiles) {
Expand Down
4 changes: 4 additions & 0 deletions src/deploy-commands.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ const path = require('path');

const commands = [];
const commandsPath = path.join(__dirname, 'commands');
if (!fs.existsSync(commandsPath)) {
console.warn(`[WARNING] Commands directory missing at ${commandsPath}. Auto-creating it.`);
fs.mkdirSync(commandsPath, { recursive: true });
Comment on lines 7 to +10
Copy link

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

deploy-commands.js is still looking in path.join(__dirname, 'commands'), but this repo’s slash commands live under src/slashcommands/. With the new auto-create behavior, a missing src/commands will now be created and commandFiles will be empty, causing the subsequent rest.put(..., { body: [] }) to overwrite/delete all registered app commands. Update the path to the real slash command directory (and avoid creating a new empty commands dir).

Suggested change
const commandsPath = path.join(__dirname, 'commands');
if (!fs.existsSync(commandsPath)) {
console.warn(`[WARNING] Commands directory missing at ${commandsPath}. Auto-creating it.`);
fs.mkdirSync(commandsPath, { recursive: true });
const commandsPath = path.join(__dirname, 'slashcommands');
if (!fs.existsSync(commandsPath)) {
console.error(`[ERROR] Slash commands directory missing at ${commandsPath}. Aborting command deployment.`);
process.exit(1);

Copilot uses AI. Check for mistakes.
Comment on lines +8 to +10
Copy link

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR title/description says "Fixes #3", but the referenced Issue #3 is about adding an /avatar slash command. This change is about directory existence checks (matching bounty-board item #21.3 instead). Please update the PR metadata (title/body/linked issue) so it closes the correct issue and doesn’t incorrectly resolve the avatar feature request.

Copilot uses AI. Check for mistakes.
}
const commandFiles = fs.readdirSync(commandsPath).filter(f => f.endsWith('.js'));
Comment on lines +8 to 12
Copy link

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider failing fast (non-zero exit) when the commands directory is missing or when no command files are found, rather than auto-creating the directory and continuing. As written, a CI/CD misconfiguration (wrong working dir, missing volume mount, etc.) can silently deploy an empty command set and wipe existing registered commands.

Copilot uses AI. Check for mistakes.

for (const file of commandFiles) {
Expand Down
Loading