Skip to content

chore: merge dev to main#52

Draft
PalmDevs wants to merge 7 commits intomainfrom
dev
Draft

chore: merge dev to main#52
PalmDevs wants to merge 7 commits intomainfrom
dev

Conversation

@PalmDevs
Copy link
Member

No description provided.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR merges dev into main, introducing a Discord reminder feature (new /remind and /unremind commands plus a ready-time polling job) and switching the bot’s SQLite setup to run Drizzle migrations automatically at startup.

Changes:

  • Add reminder commands and a background reminder dispatcher on ready.
  • Add a new reminders Drizzle schema and enable runtime DB migrations.
  • Commit/copy .drizzle artifacts, bump bot version, and update changelog/gitignore/build script.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
bots/discord/src/events/discord/ready/checkReminders.ts Polls DB for due reminders and sends them to Discord channels on an interval.
bots/discord/src/database/schemas.ts Adds reminders table schema/type.
bots/discord/src/context.ts Replaces ad-hoc schema bootstrap with Drizzle migrator-based startup.
bots/discord/src/commands/utility/remind.ts Implements /remind (create reminder or list user reminders).
bots/discord/src/commands/utility/unremind.ts Implements /unremind (delete a reminder by ID).
bots/discord/scripts/build.ts Keeps .drizzle after copying to dist.
bots/discord/package.json Version bump to 1.6.0-dev.1.
bots/discord/CHANGELOG.md Adds entries for reminder feature and auto-migration.
bots/discord/.gitignore Stops ignoring .drizzle so migrations are committed.
bots/discord/.drizzle/meta/_journal.json Adds Drizzle migration journal metadata.
bots/discord/.drizzle/meta/0000_snapshot.json Adds Drizzle schema snapshot metadata (currently missing reminders).
bots/discord/.drizzle/0000_schema.sql Adds initial migration SQL (currently missing reminders).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@PalmDevs PalmDevs requested a review from Copilot February 22, 2026 15:57
# @revanced/discord-bot [1.6.0-dev.2](https://github.com/revanced/revanced-bots/compare/@revanced/discord-bot@1.6.0-dev.1...@revanced/discord-bot@1.6.0-dev.2) (2026-02-22)

### Bug Fixes

* **bots/discord:** add missing database migration file ([f24a8fb](f24a8fb))
* **bots/discord:** off-by-one reminders query + log if query fails ([26680a9](26680a9))

### Features

* **bots/discord/database/schemas:** add indexes on reminders ([99c7422](99c7422))
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 12 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +82 to +100
const durationMs = parseDuration(interval ?? '1d', 'd')

if (durationMs <= 0 || !Number.isFinite(durationMs)) {
const embed = applyCommonEmbedStyles(
new EmbedBuilder()
.setTitle('Invalid duration')
.setDescription('Please provide a valid duration (e.g., 1d, 2h30m, 1w).')
.setColor('Red'),
false,
false,
false,
)

await interaction.reply({
embeds: [embed],
flags: MessageFlags.Ephemeral,
})
return
}
Copy link

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

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

The reminder interval validation only checks if the duration is positive and finite, but doesn't set an upper bound. Users could create reminders with extremely long intervals (e.g., years or decades), which could cause integer overflow issues when calculating nextRemindAt or create reminders that will never realistically fire. Consider adding a maximum interval constraint (e.g., 1 year) to prevent abuse and ensure reminders remain practical.

Copilot uses AI. Check for mistakes.
Comment on lines +82 to +100
const durationMs = parseDuration(interval ?? '1d', 'd')

if (durationMs <= 0 || !Number.isFinite(durationMs)) {
const embed = applyCommonEmbedStyles(
new EmbedBuilder()
.setTitle('Invalid duration')
.setDescription('Please provide a valid duration (e.g., 1d, 2h30m, 1w).')
.setColor('Red'),
false,
false,
false,
)

await interaction.reply({
embeds: [embed],
flags: MessageFlags.Ephemeral,
})
return
}
Copy link

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

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

The reminder interval validation doesn't enforce a minimum interval beyond zero. Users could create reminders with very short intervals (e.g., 1 second), leading to spam and potential rate limiting issues with Discord's API. Consider adding a minimum interval constraint (e.g., 1 minute or 5 minutes) to prevent abuse and ensure the bot remains within Discord's rate limits.

Copilot uses AI. Check for mistakes.
Comment on lines +106 to +121
const [inserted] = await database
.insert(reminders)
.values({
creatorId: interaction.user.id,
targetId: targetUser.id,
guildId: interaction.guildId!,
channelId: interaction.channelId,
message: message,
createdAt: now,
remindAt: remindAt,
intervalSeconds: intervalSeconds,
count: 0,
})
.returning()

const reminderId = inserted?.id ?? 'unknown'
Copy link

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

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

The code assumes that inserted will contain the newly created reminder with its ID, but if the insertion fails or returns undefined due to the unique constraint violations from the schema (lines 19-20 in schemas.ts), reminderId will be set to 'unknown'. This could lead to confusing log messages and user feedback. Consider adding error handling to check if the insert was successful and provide a more meaningful error message if it fails.

Copilot uses AI. Check for mistakes.
Comment on lines +20 to +22
uniqueIndex('reminders_creator_guild_idx').on(table.creatorId, table.guildId),
],
)
Copy link

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

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

The unique index on (creatorId, guildId) will prevent a user from creating more than one reminder per guild. This is overly restrictive since users should be able to have multiple active reminders in the same guild. Consider removing this unique constraint or adjusting it to allow multiple reminders per creator per guild.

Suggested change
uniqueIndex('reminders_creator_guild_idx').on(table.creatorId, table.guildId),
],
)
],
)
)

Copilot uses AI. Check for mistakes.
try {
logger.debug(`Processing reminder #${reminder.id} for ${reminder.targetId}`)

const guild = await client.guilds.fetch(reminder.guildId)
Copy link

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

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

If client.guilds.fetch() fails (e.g., bot is no longer in the guild), the error is caught and logged, but the reminder remains in the database and will be retried on the next check. Consider adding logic to delete the reminder if the guild no longer exists or is inaccessible, similar to how channel deletion is handled on lines 33-39.

Suggested change
const guild = await client.guilds.fetch(reminder.guildId)
let guild
try {
guild = await client.guilds.fetch(reminder.guildId)
} catch (e) {
logger.warn(
`Guild ${reminder.guildId} for reminder #${reminder.id} is not accessible or doesn't exist`,
)
await database.delete(reminders).where(eq(reminders.id, reminder.id))
continue
}

Copilot uses AI. Check for mistakes.
Copy link
Member Author

Choose a reason for hiding this comment

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

@oSumAtrIX Do we have any plans to support multiple servers? If not, this is trivial to implement. Will also have to consider the error reported here too, can happen if we lose connection during fetch, Discord has an outage, etc.

Copy link
Member

Choose a reason for hiding this comment

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

The bot itself is probably easier and better to support a single guild ever.

type: Command.OptionType.User,
},
},
async execute({ logger }, interaction, { message, interval, user }) {
Copy link

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

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

The command allows any user to set reminders for other users without any permission checks or restrictions. This could be used for harassment or spam by repeatedly setting unwanted reminders for other users. Consider either restricting the ability to set reminders for others to users with specific permissions, or requiring the target user's consent, or limiting the number of reminders one user can set for another.

Suggested change
async execute({ logger }, interaction, { message, interval, user }) {
async execute({ logger }, interaction, { message, interval, user }) {
// Disallow setting reminders for other users to prevent abuse/harassment
if (user && user.id !== interaction.user.id) {
await interaction.reply({
content: 'You can only set reminders for yourself.',
flags: MessageFlags.Ephemeral,
})
return
}

Copilot uses AI. Check for mistakes.
Copy link
Member Author

Choose a reason for hiding this comment

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

Additional permission checks can be added, but reminders can be set for others.

Copy link
Member Author

Choose a reason for hiding this comment

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

@oSumAtrIX Currently any user can set a reminder, this includes non-team. Should we restrict it to team-only, maybe with a configuration option?

Copy link
Member

Choose a reason for hiding this comment

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

Sure, it can be restricted to roles

@PalmDevs PalmDevs marked this pull request as draft February 22, 2026 16:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants