fix: replace broken @self.tree.interaction_check with MaintenanceAwareTree subclass
All checks were successful
Build Docker Image / build (pull_request) Successful in 1m11s
All checks were successful
Build Docker Image / build (pull_request) Successful in 1m11s
The previous code attempted to register a maintenance mode gate via @self.tree.interaction_check inside setup_hook. That pattern is invalid in discord.py — interaction_check is an overridable method on CommandTree, not a decorator. The assignment was silently dropped, making maintenance mode a no-op and producing a RuntimeWarning about an unawaited coroutine. Changes: - Add MaintenanceAwareTree(discord.app_commands.CommandTree) that overrides interaction_check: blocks non-admins when bot.maintenance_mode is True, always passes admins through, no-op when maintenance mode is off - Pass tree_cls=MaintenanceAwareTree to super().__init__() in SBABot.__init__ - Add self.maintenance_mode: bool = False to SBABot.__init__ - Update /admin-maintenance command to actually toggle bot.maintenance_mode - Add tests/test_bot_maintenance_tree.py with 8 unit tests covering all maintenance mode states, admin pass-through, DM context, and missing attr Closes #82 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
2f7b82e377
commit
d295f27afe
40
bot.py
40
bot.py
@ -65,6 +65,44 @@ def setup_logging():
|
|||||||
return logger
|
return logger
|
||||||
|
|
||||||
|
|
||||||
|
class MaintenanceAwareTree(discord.app_commands.CommandTree):
|
||||||
|
"""
|
||||||
|
CommandTree subclass that gates all interactions behind a maintenance mode check.
|
||||||
|
|
||||||
|
When bot.maintenance_mode is True, non-administrator users receive an ephemeral
|
||||||
|
error message and the interaction is blocked. Administrators are always allowed
|
||||||
|
through. When maintenance_mode is False the check is a no-op and every
|
||||||
|
interaction proceeds normally.
|
||||||
|
|
||||||
|
This is the correct way to register a global interaction_check for app commands
|
||||||
|
in discord.py — overriding the method on a CommandTree subclass passed via
|
||||||
|
tree_cls rather than attempting to assign a decorator to self.tree inside
|
||||||
|
setup_hook.
|
||||||
|
"""
|
||||||
|
|
||||||
|
async def interaction_check(self, interaction: discord.Interaction) -> bool:
|
||||||
|
"""Allow admins through; block everyone else when maintenance mode is active."""
|
||||||
|
bot = interaction.client # type: ignore[assignment]
|
||||||
|
|
||||||
|
# If maintenance mode is off, always allow.
|
||||||
|
if not getattr(bot, "maintenance_mode", False):
|
||||||
|
return True
|
||||||
|
|
||||||
|
# Maintenance mode is on — let administrators through unconditionally.
|
||||||
|
if (
|
||||||
|
isinstance(interaction.user, discord.Member)
|
||||||
|
and interaction.user.guild_permissions.administrator
|
||||||
|
):
|
||||||
|
return True
|
||||||
|
|
||||||
|
# Block non-admin users with an ephemeral notice.
|
||||||
|
await interaction.response.send_message(
|
||||||
|
"The bot is currently in maintenance mode. Please try again later.",
|
||||||
|
ephemeral=True,
|
||||||
|
)
|
||||||
|
return False
|
||||||
|
|
||||||
|
|
||||||
class SBABot(commands.Bot):
|
class SBABot(commands.Bot):
|
||||||
"""Custom bot class for SBA league management."""
|
"""Custom bot class for SBA league management."""
|
||||||
|
|
||||||
@ -78,8 +116,10 @@ class SBABot(commands.Bot):
|
|||||||
command_prefix="!", # Legacy prefix, primarily using slash commands
|
command_prefix="!", # Legacy prefix, primarily using slash commands
|
||||||
intents=intents,
|
intents=intents,
|
||||||
description="Major Domo v2.0",
|
description="Major Domo v2.0",
|
||||||
|
tree_cls=MaintenanceAwareTree,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
self.maintenance_mode: bool = False
|
||||||
self.logger = logging.getLogger("discord_bot_v2")
|
self.logger = logging.getLogger("discord_bot_v2")
|
||||||
|
|
||||||
async def setup_hook(self):
|
async def setup_hook(self):
|
||||||
|
|||||||
File diff suppressed because it is too large
Load Diff
282
tests/test_bot_maintenance_tree.py
Normal file
282
tests/test_bot_maintenance_tree.py
Normal file
@ -0,0 +1,282 @@
|
|||||||
|
"""
|
||||||
|
Tests for MaintenanceAwareTree and the maintenance_mode attribute on SBABot.
|
||||||
|
|
||||||
|
What:
|
||||||
|
Verifies that the CommandTree subclass correctly gates interactions behind
|
||||||
|
bot.maintenance_mode. When maintenance mode is off every interaction is
|
||||||
|
allowed through unconditionally. When maintenance mode is on, non-admin
|
||||||
|
users receive an ephemeral error message and the check returns False, while
|
||||||
|
administrators are always allowed through.
|
||||||
|
|
||||||
|
Why:
|
||||||
|
The original code attempted to register an interaction_check via a decorator
|
||||||
|
on self.tree inside setup_hook. That is not a valid pattern in discord.py —
|
||||||
|
interaction_check is an overridable async method on CommandTree, not a
|
||||||
|
decorator. The broken assignment caused a RuntimeWarning (unawaited
|
||||||
|
coroutine) and silently made maintenance mode a no-op. These tests confirm
|
||||||
|
the correct subclass-based implementation behaves as specified.
|
||||||
|
"""
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
from unittest.mock import AsyncMock, MagicMock
|
||||||
|
|
||||||
|
import discord
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# Helpers / fixtures
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
|
||||||
|
def _make_bot(maintenance_mode: bool = False) -> MagicMock:
|
||||||
|
"""Return a minimal mock bot with a maintenance_mode attribute."""
|
||||||
|
bot = MagicMock()
|
||||||
|
bot.maintenance_mode = maintenance_mode
|
||||||
|
return bot
|
||||||
|
|
||||||
|
|
||||||
|
def _make_interaction(is_admin: bool, bot: MagicMock) -> AsyncMock:
|
||||||
|
"""
|
||||||
|
Build a mock discord.Interaction.
|
||||||
|
|
||||||
|
The interaction's .client is set to *bot* so that MaintenanceAwareTree
|
||||||
|
can read bot.maintenance_mode via interaction.client, mirroring how
|
||||||
|
discord.py wires things at runtime.
|
||||||
|
"""
|
||||||
|
interaction = AsyncMock(spec=discord.Interaction)
|
||||||
|
interaction.client = bot
|
||||||
|
|
||||||
|
# Mock the user as a guild Member so that guild_permissions is accessible.
|
||||||
|
user = MagicMock(spec=discord.Member)
|
||||||
|
user.guild_permissions = MagicMock()
|
||||||
|
user.guild_permissions.administrator = is_admin
|
||||||
|
interaction.user = user
|
||||||
|
|
||||||
|
# response.send_message must be awaitable.
|
||||||
|
interaction.response = AsyncMock()
|
||||||
|
interaction.response.send_message = AsyncMock()
|
||||||
|
|
||||||
|
return interaction
|
||||||
|
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# Import the class under test after mocks are available.
|
||||||
|
# We import here (not at module level) so that the conftest env-vars are set
|
||||||
|
# before any discord_bot_v2 modules are touched.
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.fixture(autouse=True)
|
||||||
|
def _patch_discord_app_commands(monkeypatch):
|
||||||
|
"""
|
||||||
|
Prevent MaintenanceAwareTree.__init__ from calling discord internals that
|
||||||
|
need a real event loop / Discord connection. We test only the logic of
|
||||||
|
interaction_check, so we stub out the parent __init__.
|
||||||
|
"""
|
||||||
|
# Nothing extra to patch for the interaction_check itself; the parent
|
||||||
|
# CommandTree.__init__ is only called when constructing SBABot, which we
|
||||||
|
# don't do in these unit tests.
|
||||||
|
yield
|
||||||
|
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# Tests for MaintenanceAwareTree.interaction_check
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
|
||||||
|
class TestMaintenanceAwareTree:
|
||||||
|
"""Unit tests for MaintenanceAwareTree.interaction_check."""
|
||||||
|
|
||||||
|
@pytest.fixture
|
||||||
|
def tree_cls(self):
|
||||||
|
"""Import and return the MaintenanceAwareTree class."""
|
||||||
|
from bot import MaintenanceAwareTree
|
||||||
|
|
||||||
|
return MaintenanceAwareTree
|
||||||
|
|
||||||
|
# ------------------------------------------------------------------
|
||||||
|
# Maintenance OFF
|
||||||
|
# ------------------------------------------------------------------
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_maintenance_off_allows_non_admin(self, tree_cls):
|
||||||
|
"""
|
||||||
|
When maintenance_mode is False, non-admin users are always allowed.
|
||||||
|
The check must return True without sending any message.
|
||||||
|
"""
|
||||||
|
bot = _make_bot(maintenance_mode=False)
|
||||||
|
interaction = _make_interaction(is_admin=False, bot=bot)
|
||||||
|
|
||||||
|
# Instantiate tree without calling parent __init__ by testing the method
|
||||||
|
# directly on an unbound basis.
|
||||||
|
result = await tree_cls.interaction_check(
|
||||||
|
MagicMock(), # placeholder 'self' for the tree instance
|
||||||
|
interaction,
|
||||||
|
)
|
||||||
|
|
||||||
|
assert result is True
|
||||||
|
interaction.response.send_message.assert_not_called()
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_maintenance_off_allows_admin(self, tree_cls):
|
||||||
|
"""
|
||||||
|
When maintenance_mode is False, admin users are also always allowed.
|
||||||
|
"""
|
||||||
|
bot = _make_bot(maintenance_mode=False)
|
||||||
|
interaction = _make_interaction(is_admin=True, bot=bot)
|
||||||
|
|
||||||
|
result = await tree_cls.interaction_check(MagicMock(), interaction)
|
||||||
|
|
||||||
|
assert result is True
|
||||||
|
interaction.response.send_message.assert_not_called()
|
||||||
|
|
||||||
|
# ------------------------------------------------------------------
|
||||||
|
# Maintenance ON — non-admin
|
||||||
|
# ------------------------------------------------------------------
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_maintenance_on_blocks_non_admin(self, tree_cls):
|
||||||
|
"""
|
||||||
|
When maintenance_mode is True, non-admin users must be blocked.
|
||||||
|
The check must return False and send an ephemeral message.
|
||||||
|
"""
|
||||||
|
bot = _make_bot(maintenance_mode=True)
|
||||||
|
interaction = _make_interaction(is_admin=False, bot=bot)
|
||||||
|
|
||||||
|
result = await tree_cls.interaction_check(MagicMock(), interaction)
|
||||||
|
|
||||||
|
assert result is False
|
||||||
|
interaction.response.send_message.assert_called_once()
|
||||||
|
|
||||||
|
# Confirm the call used ephemeral=True
|
||||||
|
_, kwargs = interaction.response.send_message.call_args
|
||||||
|
assert kwargs.get("ephemeral") is True
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_maintenance_on_message_has_no_emoji(self, tree_cls):
|
||||||
|
"""
|
||||||
|
The maintenance block message must not contain emoji characters.
|
||||||
|
The project style deliberately strips emoji from user-facing strings.
|
||||||
|
"""
|
||||||
|
import unicodedata
|
||||||
|
|
||||||
|
bot = _make_bot(maintenance_mode=True)
|
||||||
|
interaction = _make_interaction(is_admin=False, bot=bot)
|
||||||
|
|
||||||
|
await tree_cls.interaction_check(MagicMock(), interaction)
|
||||||
|
|
||||||
|
args, _ = interaction.response.send_message.call_args
|
||||||
|
message_text = args[0] if args else ""
|
||||||
|
|
||||||
|
for ch in message_text:
|
||||||
|
category = unicodedata.category(ch)
|
||||||
|
assert category != "So", (
|
||||||
|
f"Unexpected emoji/symbol character {ch!r} (category {category!r}) "
|
||||||
|
f"found in maintenance message: {message_text!r}"
|
||||||
|
)
|
||||||
|
|
||||||
|
# ------------------------------------------------------------------
|
||||||
|
# Maintenance ON — admin
|
||||||
|
# ------------------------------------------------------------------
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_maintenance_on_allows_admin(self, tree_cls):
|
||||||
|
"""
|
||||||
|
When maintenance_mode is True, administrator users must still be
|
||||||
|
allowed through. Admins should never be locked out of bot commands.
|
||||||
|
"""
|
||||||
|
bot = _make_bot(maintenance_mode=True)
|
||||||
|
interaction = _make_interaction(is_admin=True, bot=bot)
|
||||||
|
|
||||||
|
result = await tree_cls.interaction_check(MagicMock(), interaction)
|
||||||
|
|
||||||
|
assert result is True
|
||||||
|
interaction.response.send_message.assert_not_called()
|
||||||
|
|
||||||
|
# ------------------------------------------------------------------
|
||||||
|
# Edge case: non-Member user during maintenance
|
||||||
|
# ------------------------------------------------------------------
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_maintenance_on_blocks_non_member_user(self, tree_cls):
|
||||||
|
"""
|
||||||
|
When maintenance_mode is True and the user is not a guild Member
|
||||||
|
(e.g. interaction from a DM context), the check must still block them
|
||||||
|
because we cannot verify administrator status.
|
||||||
|
"""
|
||||||
|
bot = _make_bot(maintenance_mode=True)
|
||||||
|
interaction = AsyncMock(spec=discord.Interaction)
|
||||||
|
interaction.client = bot
|
||||||
|
|
||||||
|
# Simulate a non-Member user (e.g. discord.User from DM context)
|
||||||
|
user = MagicMock(spec=discord.User)
|
||||||
|
# discord.User has no guild_permissions attribute
|
||||||
|
interaction.user = user
|
||||||
|
interaction.response = AsyncMock()
|
||||||
|
interaction.response.send_message = AsyncMock()
|
||||||
|
|
||||||
|
result = await tree_cls.interaction_check(MagicMock(), interaction)
|
||||||
|
|
||||||
|
assert result is False
|
||||||
|
interaction.response.send_message.assert_called_once()
|
||||||
|
|
||||||
|
# ------------------------------------------------------------------
|
||||||
|
# Missing attribute safety: bot without maintenance_mode attr
|
||||||
|
# ------------------------------------------------------------------
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_missing_maintenance_mode_attr_defaults_to_allowed(self, tree_cls):
|
||||||
|
"""
|
||||||
|
If the bot object does not have a maintenance_mode attribute (e.g.
|
||||||
|
during testing or unusual startup), getattr fallback must treat it as
|
||||||
|
False and allow the interaction.
|
||||||
|
"""
|
||||||
|
bot = MagicMock()
|
||||||
|
# Deliberately do NOT set bot.maintenance_mode
|
||||||
|
del bot.maintenance_mode
|
||||||
|
|
||||||
|
interaction = _make_interaction(is_admin=False, bot=bot)
|
||||||
|
|
||||||
|
result = await tree_cls.interaction_check(MagicMock(), interaction)
|
||||||
|
|
||||||
|
assert result is True
|
||||||
|
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# Tests for SBABot.maintenance_mode attribute
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
|
||||||
|
class TestSBABotMaintenanceModeAttribute:
|
||||||
|
"""
|
||||||
|
Confirms that SBABot.__init__ always sets maintenance_mode = False.
|
||||||
|
|
||||||
|
We avoid constructing a real SBABot (which requires a Discord event loop
|
||||||
|
and valid token infrastructure) by patching the entire commands.Bot.__init__
|
||||||
|
and then calling SBABot.__init__ directly on a bare instance so that only
|
||||||
|
the SBABot-specific attribute assignments execute.
|
||||||
|
"""
|
||||||
|
|
||||||
|
def test_maintenance_mode_default_is_false(self, monkeypatch):
|
||||||
|
"""
|
||||||
|
SBABot.__init__ must set self.maintenance_mode = False so that the
|
||||||
|
MaintenanceAwareTree has the attribute available from the very first
|
||||||
|
interaction, even before /admin-maintenance is ever called.
|
||||||
|
|
||||||
|
Strategy: patch commands.Bot.__init__ to be a no-op so super().__init__
|
||||||
|
succeeds without a real Discord connection, then call SBABot.__init__
|
||||||
|
and assert the attribute is present with the correct default value.
|
||||||
|
"""
|
||||||
|
from unittest.mock import patch
|
||||||
|
from discord.ext import commands
|
||||||
|
from bot import SBABot
|
||||||
|
|
||||||
|
with patch.object(commands.Bot, "__init__", return_value=None):
|
||||||
|
bot = SBABot.__new__(SBABot)
|
||||||
|
SBABot.__init__(bot)
|
||||||
|
|
||||||
|
assert hasattr(
|
||||||
|
bot, "maintenance_mode"
|
||||||
|
), "SBABot must define self.maintenance_mode in __init__"
|
||||||
|
assert (
|
||||||
|
bot.maintenance_mode is False
|
||||||
|
), "SBABot.maintenance_mode must default to False"
|
||||||
Loading…
Reference in New Issue
Block a user