fix: explicit DM router and admin session exclusivity (#111, #112)

tests/test_admin_commands.py

@@ -276,63 +276,104 @@ def test_cooldown_expires(self, workflow_state):
         assert remaining == 0.0
 
 
-class TestOnMessageListener:
-    """Test suite for on_message DM listener."""
+class TestAdminSessionExclusivity:
+    """Admin DM workflows are mutually exclusive — only one active session per user."""
 
     @pytest.fixture
     def admin_cog(self, mock_bot, database):
         mock_bot.db = database
         return AdminCommands(mock_bot)
 
     @pytest.mark.asyncio
-    async def test_ignores_bot_messages(self, admin_cog):
-        """Bot messages are ignored to prevent infinite loops."""
-        mock_message = MagicMock()
-        mock_message.author.bot = True
-        mock_message.guild = None
+    async def test_fixture_create_command_blocked_when_results_session_active(
+        self, admin_cog, mock_interaction_admin
+    ):
+        """fixture_create gives a single clear error — not 'Check your DMs' + error."""
+        user_id = str(mock_interaction_admin.user.id)
+        mock_interaction_admin.channel_id = int(mock_interaction_admin.channel.id)
+        mock_interaction_admin.guild_id = mock_interaction_admin.guild.id
+        admin_cog.results_handler.start_session(user_id, 1, 111111, week_number=1)
 
-        result = await admin_cog.on_message(mock_message)
-        assert result is None
+        await admin_cog.fixture_create.callback(admin_cog, mock_interaction_admin)
+
+        assert len(mock_interaction_admin.response_sent) == 1
+        response = mock_interaction_admin.response_sent[0]["content"]
+        assert "results entry" in response.lower()
+        assert "Check your DMs" not in response
+        assert not admin_cog.fixture_handler.has_session(user_id)
 
     @pytest.mark.asyncio
-    async def test_ignores_guild_messages(self, admin_cog):
-        """Guild messages are ignored - admin workflows require DMs."""
-        mock_message = MagicMock()
-        mock_message.guild = MagicMock()  # Has guild
+    async def test_start_fixture_dm_blocked_when_results_session_active(
+        self, admin_cog, mock_interaction_admin
+    ):
+        """_start_fixture_dm fallback guard covers the view-triggered path."""
+        user_id = str(mock_interaction_admin.user.id)
+        admin_cog.results_handler.start_session(user_id, 1, 111111, week_number=1)
+
+        result = await admin_cog._start_fixture_dm(
+            mock_interaction_admin.user,
+            user_id,
+            channel_id=123456,
+            guild_id=111111,
+        )
 
-        result = await admin_cog.on_message(mock_message)
-        assert result is None
+        assert result is False
+        assert not admin_cog.fixture_handler.has_session(user_id)
+        assert len(mock_interaction_admin.user.dm_sent) == 1
+        assert "results entry" in mock_interaction_admin.user.dm_sent[0].lower()
 
     @pytest.mark.asyncio
-    async def test_handles_fixture_creation_dm(self, admin_cog):
-        """Fixture creation DMs route to the correct handler."""
-        mock_message = MagicMock()
-        mock_message.guild = None
-        user_id = "123456"
-        mock_message.author.id = 123456
-        mock_message.author.bot = False
+    async def test_results_enter_blocked_when_fixture_session_active(
+        self, admin_cog, mock_interaction_admin, sample_games
+    ):
+        """Starting results entry while fixture creation is in progress is rejected."""
+        from datetime import UTC, datetime, timedelta
 
+        deadline = datetime.now(UTC) + timedelta(days=1)
+        await admin_cog.db.create_fixture(1, sample_games, deadline)
+
+        user_id = str(mock_interaction_admin.user.id)
+        mock_interaction_admin.guild_id = mock_interaction_admin.guild.id
         admin_cog.fixture_handler.start_session(user_id, 123456, 111111)
-        admin_cog.fixture_handler.handle_dm = AsyncMock(return_value=True)
 
-        await admin_cog.on_message(mock_message)
+        await admin_cog.results_enter.callback(admin_cog, mock_interaction_admin, 1)
+
+        assert not admin_cog.results_handler.has_session(user_id)
+        response = mock_interaction_admin.response_sent[-1]
+        assert "fixture creation" in response["content"].lower()
+
+    @pytest.mark.asyncio
+    async def test_fixture_create_allowed_when_no_conflicting_session(
+        self, admin_cog, mock_interaction_admin
+    ):
+        """Fixture creation proceeds normally when no admin session is active."""
+        user_id = str(mock_interaction_admin.user.id)
+
+        result = await admin_cog._start_fixture_dm(
+            mock_interaction_admin.user,
+            user_id,
+            channel_id=123456,
+            guild_id=111111,
+        )
 
+        assert result is True
         assert admin_cog.fixture_handler.has_session(user_id)
 
     @pytest.mark.asyncio
-    async def test_handles_results_entry_dm(self, admin_cog):
-        """Results entry DMs route to the correct handler."""
-        mock_message = MagicMock()
-        mock_message.guild = None
-        user_id = "123456"
-        mock_message.author.id = 123456
-        mock_message.author.bot = False
+    async def test_results_enter_allowed_when_no_conflicting_session(
+        self, admin_cog, mock_interaction_admin, sample_games
+    ):
+        """Results entry proceeds normally when no admin session is active."""
+        from datetime import UTC, datetime, timedelta
 
-        admin_cog.results_handler.start_session(user_id, 1, 111111, week_number=1)
-        admin_cog.results_handler.handle_dm = AsyncMock(return_value=True)
+        deadline = datetime.now(UTC) + timedelta(days=1)
+        await admin_cog.db.create_fixture(1, sample_games, deadline)
 
-        await admin_cog.on_message(mock_message)
+        mock_interaction_admin.guild_id = mock_interaction_admin.guild.id
 
+        await admin_cog.results_enter.callback(admin_cog, mock_interaction_admin, 1)
+
+        user_id = str(mock_interaction_admin.user.id)
         assert admin_cog.results_handler.has_session(user_id)
 
 

tests/test_bot.py

@@ -48,9 +48,16 @@ class TestSetupHook:
     async def bot_instance(self):
         mock_tree = MagicMock()
         mock_tree.sync = AsyncMock(return_value=[])
+        mock_admin_cog = MagicMock()
+        mock_admin_cog.fixture_handler = MagicMock()
+        mock_admin_cog.results_handler = MagicMock()
+        mock_user_cog = MagicMock()
+        mock_user_cog.prediction_handler = MagicMock()
+        mock_cogs = {"AdminCommands": mock_admin_cog, "UserCommands": mock_user_cog}
         with (
             patch("typer_bot.bot.commands.Bot.__init__", return_value=None),
             patch.object(TyperBot, "tree", mock_tree),
+            patch.object(TyperBot, "cogs", mock_cogs),
         ):
             bot = TyperBot.__new__(TyperBot)
             bot.db = MagicMock()
@@ -421,6 +428,72 @@ async def test_on_message_sets_trace_id(self, bot_instance):
             mock_set_trace.assert_called_once_with("msg-123456")
 
 
+class TestOnMessageDMRouting:
+    """Test suite verifying DM messages are routed through DMRouter."""
+
+    @pytest.fixture
+    def bot_instance(self):
+        mock_router = MagicMock()
+        mock_router.route = AsyncMock(return_value=True)
+        with patch("typer_bot.bot.commands.Bot.__init__", return_value=None):
+            bot = TyperBot.__new__(TyperBot)
+            bot.thread_handler = MagicMock()
+            bot.thread_handler.on_message = AsyncMock(return_value=False)
+            bot.dm_router = mock_router
+            yield bot
+
+    @pytest.mark.asyncio
+    async def test_dm_routes_through_dm_router(self, bot_instance):
+        """DMs are dispatched to the router, not to cog listeners."""
+        mock_message = MagicMock()
+        mock_message.author.bot = False
+        mock_message.guild = None
+        mock_message.id = 1
+
+        await bot_instance.on_message(mock_message)
+
+        bot_instance.dm_router.route.assert_awaited_once_with(mock_message)
+
+    @pytest.mark.asyncio
+    async def test_guild_messages_skip_dm_router(self, bot_instance):
+        """Guild messages go through normal command processing, not the DM router."""
+        mock_message = MagicMock()
+        mock_message.author.bot = False
+        mock_message.guild = MagicMock()
+        mock_message.id = 2
+
+        with patch("discord.ext.commands.Bot.on_message", new_callable=AsyncMock):
+            await bot_instance.on_message(mock_message)
+
+        bot_instance.dm_router.route.assert_not_awaited()
+
+    @pytest.mark.asyncio
+    async def test_none_router_logs_warning_and_drops_dm(self, bot_instance):
+        """DMs received before the router is ready are logged and dropped."""
+        bot_instance.dm_router = None
+        mock_message = MagicMock()
+        mock_message.author.bot = False
+        mock_message.guild = None
+        mock_message.id = 3
+
+        with patch("typer_bot.bot.logger") as mock_logger:
+            await bot_instance.on_message(mock_message)
+            mock_logger.warning.assert_called_once()
+
+    @pytest.mark.asyncio
+    async def test_thread_handler_takes_priority_over_dm_router(self, bot_instance):
+        """Thread messages are consumed before reaching the DM router."""
+        bot_instance.thread_handler.on_message = AsyncMock(return_value=True)
+        mock_message = MagicMock()
+        mock_message.author.bot = False
+        mock_message.guild = None
+        mock_message.id = 4
+
+        await bot_instance.on_message(mock_message)
+
+        bot_instance.dm_router.route.assert_not_awaited()
+
+
 class TestOnInteraction:
     """Test suite for on_interaction event handler."""
 

tests/test_dm_prediction_handler.py

@@ -31,20 +31,6 @@ async def test_ignores_guild_messages(self, prediction_handler, mock_message):
         assert not handled
         assert len(mock_message.author.dm_sent) == 0
 
-    @pytest.mark.asyncio
-    async def test_ignores_dms_during_results_entry(
-        self, prediction_handler, mock_message, workflow_state
-    ):
-        mock_message.guild = None
-        user_id = str(mock_message.author.id)
-        session = workflow_state.start_results_session(user_id, 1, 123456)
-        session.created_at = datetime.now(UTC)
-
-        handled = await prediction_handler.handle_dm(mock_message)
-
-        assert not handled
-        assert len(mock_message.author.dm_sent) == 0
-
     @pytest.mark.asyncio
     async def test_rejects_message_too_long(self, prediction_handler, mock_message):
         mock_message.guild = None

tests/test_dm_router.py

@@ -0,0 +1,125 @@
+"""Tests for DM routing precedence."""
+
+from unittest.mock import AsyncMock, MagicMock
+
+import pytest
+
+from typer_bot.services.dm_router import DMRouter
+
+
+def _make_dm_message(user_id: str = "123456", bot: bool = False, in_guild: bool = False):
+    message = MagicMock()
+    message.author.id = int(user_id)
+    message.author.bot = bot
+    message.guild = MagicMock() if in_guild else None
+    return message
+
+
+@pytest.fixture
+def fixture_handler():
+    h = MagicMock()
+    h.has_session.return_value = False
+    h.handle_dm = AsyncMock(return_value=True)
+    return h
+
+
+@pytest.fixture
+def results_handler():
+    h = MagicMock()
+    h.has_session.return_value = False
+    h.handle_dm = AsyncMock(return_value=True)
+    return h
+
+
+@pytest.fixture
+def prediction_handler():
+    h = MagicMock()
+    h.handle_dm = AsyncMock(return_value=True)
+    return h
+
+
+@pytest.fixture
+def router(fixture_handler, results_handler, prediction_handler):
+    return DMRouter(fixture_handler, results_handler, prediction_handler)
+
+
+class TestRouterIgnoresNonDMs:
+    @pytest.mark.asyncio
+    async def test_ignores_bot_messages(self, router):
+        result = await router.route(_make_dm_message(bot=True))
+        assert result is False
+
+    @pytest.mark.asyncio
+    async def test_ignores_guild_messages(self, router):
+        result = await router.route(_make_dm_message(in_guild=True))
+        assert result is False
+
+
+class TestRoutingPrecedence:
+    @pytest.mark.asyncio
+    async def test_fixture_session_routes_to_fixture_handler(
+        self, router, fixture_handler, results_handler, prediction_handler
+    ):
+        fixture_handler.has_session.return_value = True
+        message = _make_dm_message()
+
+        result = await router.route(message)
+
+        assert result is True
+        fixture_handler.handle_dm.assert_awaited_once()
+        results_handler.handle_dm.assert_not_awaited()
+        prediction_handler.handle_dm.assert_not_awaited()
+
+    @pytest.mark.asyncio
+    async def test_results_session_routes_to_results_handler(
+        self, router, fixture_handler, results_handler, prediction_handler
+    ):
+        results_handler.has_session.return_value = True
+        message = _make_dm_message()
+
+        result = await router.route(message)
+
+        assert result is True
+        results_handler.handle_dm.assert_awaited_once()
+        fixture_handler.handle_dm.assert_not_awaited()
+        prediction_handler.handle_dm.assert_not_awaited()
+
+    @pytest.mark.asyncio
+    async def test_no_admin_session_falls_through_to_prediction(
+        self, router, fixture_handler, results_handler, prediction_handler
+    ):
+        message = _make_dm_message()
+
+        result = await router.route(message)
+
+        assert result is True
+        prediction_handler.handle_dm.assert_awaited_once_with(message)
+        fixture_handler.handle_dm.assert_not_awaited()
+        results_handler.handle_dm.assert_not_awaited()
+
+    @pytest.mark.asyncio
+    async def test_fixture_session_takes_precedence_over_results(
+        self, router, fixture_handler, results_handler
+    ):
+        """Fixture check runs first; results handler should never be reached."""
+        fixture_handler.has_session.return_value = True
+        results_handler.has_session.return_value = True
+        message = _make_dm_message()
+
+        await router.route(message)
+
+        fixture_handler.handle_dm.assert_awaited_once()
+        results_handler.handle_dm.assert_not_awaited()
+
+    @pytest.mark.asyncio
+    async def test_admin_session_takes_precedence_over_prediction(
+        self, router, results_handler, prediction_handler
+    ):
+        """Any active admin session blocks the prediction handler."""
+        results_handler.has_session.return_value = True
+        message = _make_dm_message()
+
+        await router.route(message)
+
+        results_handler.handle_dm.assert_awaited_once()
+        prediction_handler.handle_dm.assert_not_awaited()

tests/test_user_commands.py

@@ -14,17 +14,6 @@ async def user_commands(mock_bot, database):
     return UserCommands(mock_bot)
 
 
-class TestOnMessage:
-    @pytest.mark.asyncio
-    async def test_delegates_dm_messages_to_prediction_handler(self, user_commands, mock_message):
-        handler = AsyncMock(return_value=True)
-        user_commands.prediction_handler.handle_dm = handler
-
-        await user_commands.on_message(mock_message)
-
-        handler.assert_awaited_once_with(mock_message)
-
-
 class TestPredictCommand:
     @pytest.mark.asyncio
     async def test_no_fixture_shows_error(self, user_commands, mock_interaction):