Multiple off-by-one errors and silent reminder deletion with no user notification in reminders.py

[oliveman-au]

The reminder limit for regular users is defined as:

MAXIMUM_REMINDERS = 5

However, the check in new_reminder uses a strict greater-than comparison:

if len(active_reminders) > MAXIMUM_REMINDERS:
    await send_denial(ctx, "You have too many active reminders!")
    return

Because > 5 only triggers when a user already has 6 reminders, a user can successfully create a 6th reminder before being blocked.

The fix would be changing > to >=.

Steps to reproduce (as a regular member in #bot-commands):

1. Run !remind new 7d reminder 1 through !remind new 7d reminder 6 six times
2. All six succeed — the denial is never triggered

Expected: denial on the 6th attempt
Reproduced on: bot/exts/utils/reminders.py, line 478

Proof shown below in images or can be recreated with steps above

[oliveman-au]

Additional bugs found in bot/exts/utils/reminders.py

---

Bug 1: Off-by-one — users can create 6 reminders despite a limit of 5

Current code (reminders.py line 478):

MAXIMUM_REMINDERS = 5

if len(active_reminders) > MAXIMUM_REMINDERS:
    await send_denial(ctx, "You have too many active reminders!")
    return

> 5 only evaluates to True once the user already has 6 reminders in the database — meaning the 6th is always written before the check can block it. The denial never fires on the 6th attempt.

Fix:

if len(active_reminders) >= MAXIMUM_REMINDERS:
    await send_denial(ctx, "You have too many active reminders!")
    return

Steps to reproduce (as a regular member in #bot-commands):

1. Run each of the following one at a time:

!remind new 7d reminder 1
!remind new 7d reminder 2
!remind new 7d reminder 3
!remind new 7d reminder 4
!remind new 7d reminder 5
!remind new 7d reminder 6

2. All 6 succeed with a confirmation embed — the denial is never shown
3. Run !remind list to confirm all 6 are saved

Expected: the bot sends a denial on the 6th attempt and does not save it

---

Bug 2: Off-by-one — users can delete 6 reminders at once despite a limit of 5

The docstring explicitly states the limit is 5, but the same > vs >= mistake is present:

Current code (reminders.py line 660-663):

async def delete_reminder(self, ctx: Context, ids: Greedy[int]) -> None:
    """Delete up to (and including) 5 of your active reminders."""
    if len(ids) > 5:
        await send_denial(ctx, "You can only delete a maximum of 5 reminders at once.")
        return

> 5 is only True when 6 or more IDs are passed, so passing exactly 6 IDs is accepted and all 6 are deleted. The docstring and the behaviour directly contradict each other.

Fix:

async def delete_reminder(self, ctx: Context, ids: Greedy[int]) -> None:
    """Delete up to (and including) 5 of your active reminders."""
    if len(ids) >= 5:
        await send_denial(ctx, "You can only delete a maximum of 5 reminders at once.")
        return

Steps to reproduce (as a regular member in #bot-commands):

1. First create 6 reminders using the steps in Bug 1 above
2. Run !remind list and copy the 6 reminder IDs
3. Run:

!remind delete <id1> <id2> <id3> <id4> <id5> <id6>

4. The bot confirms all 6 were deleted — no denial is shown

Expected: the bot sends a denial when more than 5 IDs are provided and deletes nothing

---

Bug 3: Reminders are silently deleted with no user notification when the bot loses channel cache

Current code (reminders.py line 248-258):

def ensure_valid_reminder(self, reminder: dict) -> tuple[bool, discord.TextChannel]:
    """Ensure reminder channel can be fetched otherwise delete the reminder."""
    channel = self.bot.get_channel(reminder["channel_id"])
    is_valid = True
    if not channel:
        is_valid = False
        log.info(
            f"Reminder {reminder['id']} invalid: "
            f"Channel {reminder['channel_id']}={channel}."
        )
        scheduling.create_task(self.bot.api_client.delete(f"bot/reminders/{reminder['id']}"))

    return is_valid, channel

self.bot.get_channel() only checks the local cache — it does not make a Discord API call. If the bot restarts or a channel temporarily falls out of cache during an outage, get_channel() returns None even though the channel still exists and is perfectly valid. The reminder is then permanently deleted from the database with no DM, no message, and no indication to the user that their reminder was lost. The content is gone with no way to recover it.

Fix: use fetch_channel() to fall back to the Discord API before giving up, and DM the user if the reminder truly cannot be delivered:

async def ensure_valid_reminder(self, reminder: dict) -> tuple[bool, discord.TextChannel | None]:
    """Ensure reminder channel can be fetched otherwise notify the user and delete the reminder."""
    channel = self.bot.get_channel(reminder["channel_id"])
    if not channel:
        try:
            channel = await self.bot.fetch_channel(reminder["channel_id"])
        except discord.HTTPException:
            log.info(
                f"Reminder {reminder['id']} invalid: "
                f"Channel {reminder['channel_id']} could not be fetched."
            )
            user = self.bot.get_user(reminder["author"])
            if user:
                await user.send(
                    f"Your reminder could not be delivered because the channel it was set in is no longer accessible.\n"
                    f"**Reminder content:** {reminder['content']}"
                )
            scheduling.create_task(self.bot.api_client.delete(f"bot/reminders/{reminder['id']}"))
            return False, None

    return True, channel

Steps to reproduce:

1. Set a reminder with !remind new 10m check this
2. Have the bot restart before the 10 minutes are up
3. When the reminder time arrives, nothing happens — the reminder was silently deleted on restart and the user receives no notification

Expected: the user receives a DM with their reminder content explaining it could not be delivered, so they can recreate it

[oliveman-au]

I'd love to open a PR to fix all of the bugs outlined above if I'm able to be assigned. I'm happy to provide image proof reproducing each one if needed before any work begins. Thanks for your time!

[wookie184]

1 and 2 are simple and sound good.

For 3, generally all channels should be cached, so it isn't a massive issue exactly as described, but it is still an issue as the channel may simply not exist. This has an existing issue at #3015. I think DMing sounds sensible. People can also react to reminders to also be reminded, should they be DMed as well?

Also, we have a utility for fetching channel if not cached, https://bot-core.pythondiscord.com/output/pydis_core.utils.channel.html#pydis_core.utils.channel.get_or_fetch_channel, you can search the codebase for examples of how it's used.

Overall sounds good, I'll approve and assign you.

[oliveman-au]

PR can be found here - #3509