From d55bab7b63ca8ad967574ab2890f1ae939d7ef9e Mon Sep 17 00:00:00 2001 From: Jeeves Assistant Date: Thu, 18 Jun 2026 17:35:41 -0500 Subject: [PATCH 1/2] fix(discord): reuse existing auto-thread on create race --- plugins/platforms/discord/adapter.py | 21 +++++++++++++++++++ .../gateway/test_discord_channel_controls.py | 2 +- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/plugins/platforms/discord/adapter.py b/plugins/platforms/discord/adapter.py index 5fab2307c30a..4bf12e802473 100644 --- a/plugins/platforms/discord/adapter.py +++ b/plugins/platforms/discord/adapter.py @@ -6579,6 +6579,27 @@ async def _auto_create_thread(self, message: 'DiscordMessage') -> Optional[Any]: return thread except Exception as direct_error: last_direct_error = direct_error + # If another bot/client won the race and already created the + # thread from this starter message, reuse it instead of + # posting a fallback seed and creating a duplicate thread. + existing_thread = getattr(message, "thread", None) + if existing_thread is not None: + return existing_thread + if self._client is not None: + try: + existing_thread = self._client.get_channel(int(message.id)) + if existing_thread is None: + existing_thread = await self._client.fetch_channel(int(message.id)) + if existing_thread is not None: + return existing_thread + except Exception: + logger.debug( + "[%s] Could not resolve existing Discord thread for starter message %s", + self.name, + getattr(message, "id", "unknown"), + exc_info=True, + ) + try: seed_msg = await message.channel.send( f"\U0001f9f5 Thread created by Hermes: **{thread_name}**" diff --git a/tests/gateway/test_discord_channel_controls.py b/tests/gateway/test_discord_channel_controls.py index 03a210360b0a..009031f8b8c9 100644 --- a/tests/gateway/test_discord_channel_controls.py +++ b/tests/gateway/test_discord_channel_controls.py @@ -61,6 +61,7 @@ def __init__(self, channel_id: int = 1, name: str = "general", guild_name: str = self.name = name self.guild = SimpleNamespace(name=guild_name) self.topic = None + self.send = AsyncMock() class FakeThread: @@ -239,4 +240,3 @@ def test_config_bridges_ignored_channels(monkeypatch, tmp_path): import os assert os.getenv("DISCORD_IGNORED_CHANNELS") == "111,222" - From 39486ff3ae42923088a72a5bebf6564b65b25d37 Mon Sep 17 00:00:00 2001 From: izumi0uu Date: Tue, 28 Jul 2026 11:10:06 +0800 Subject: [PATCH 2/2] fix(discord): reconcile ambiguous auto-thread creation --- plugins/platforms/discord/adapter.py | 92 +++++++++++---- .../gateway/test_discord_channel_controls.py | 2 +- tests/gateway/test_discord_slash_commands.py | 107 +++++++++++++++++- 3 files changed, 178 insertions(+), 23 deletions(-) diff --git a/plugins/platforms/discord/adapter.py b/plugins/platforms/discord/adapter.py index 4bf12e802473..1b004c95aa4b 100644 --- a/plugins/platforms/discord/adapter.py +++ b/plugins/platforms/discord/adapter.py @@ -6553,6 +6553,56 @@ def _derive_auto_thread_name(self, content: str) -> str: thread_name = thread_name[:77] + "..." return thread_name + @staticmethod + def _is_starter_message_thread(candidate: Any, message_id: Any) -> bool: + """Return whether ``candidate`` is the thread backed by ``message_id``.""" + if candidate is None or message_id is None: + return False + if str(getattr(candidate, "id", "")) != str(message_id): + return False + + thread_type = getattr(discord, "Thread", None) + return not isinstance(thread_type, type) or isinstance(candidate, thread_type) + + async def _resolve_starter_message_thread( + self, + message: 'DiscordMessage', + ) -> Tuple[Optional[Any], bool]: + """Return an existing starter thread and whether absence was confirmed.""" + message_id = getattr(message, "id", None) + existing_thread = getattr(message, "thread", None) + if self._is_starter_message_thread(existing_thread, message_id): + return existing_thread, False + + client = self._client + if client is None or message_id is None: + return None, False + + try: + get_channel = getattr(client, "get_channel", None) + existing_thread = get_channel(int(message_id)) if callable(get_channel) else None + if self._is_starter_message_thread(existing_thread, message_id): + return existing_thread, False + + fetch_channel = getattr(client, "fetch_channel", None) + if callable(fetch_channel): + existing_thread = await fetch_channel(int(message_id)) + if self._is_starter_message_thread(existing_thread, message_id): + return existing_thread, False + if existing_thread is None: + return None, True + except Exception as exc: + not_found_type = getattr(discord, "NotFound", None) + if isinstance(not_found_type, type) and isinstance(exc, not_found_type): + return None, True + logger.debug( + "[%s] Could not resolve existing Discord thread for starter message %s", + self.name, + message_id, + exc_info=True, + ) + return None, False + async def _auto_create_thread(self, message: 'DiscordMessage') -> Optional[Any]: """Create a thread from a user message for auto-threading. @@ -6579,26 +6629,28 @@ async def _auto_create_thread(self, message: 'DiscordMessage') -> Optional[Any]: return thread except Exception as direct_error: last_direct_error = direct_error - # If another bot/client won the race and already created the - # thread from this starter message, reuse it instead of - # posting a fallback seed and creating a duplicate thread. - existing_thread = getattr(message, "thread", None) - if existing_thread is not None: - return existing_thread - if self._client is not None: - try: - existing_thread = self._client.get_channel(int(message.id)) - if existing_thread is None: - existing_thread = await self._client.fetch_channel(int(message.id)) - if existing_thread is not None: - return existing_thread - except Exception: - logger.debug( - "[%s] Could not resolve existing Discord thread for starter message %s", - self.name, - getattr(message, "id", "unknown"), - exc_info=True, - ) + # A transport error can arrive after Discord committed the + # create. Give both the gateway cache and REST lookup a brief + # chance to expose that thread before creating a fallback. + absence_confirmed = False + for reconciliation_attempt in range(2): + existing_thread, absence_confirmed = await self._resolve_starter_message_thread(message) + if existing_thread is not None: + return existing_thread + if reconciliation_attempt == 0: + await asyncio.sleep(0.25) + + if not absence_confirmed: + if attempt == 0: + await asyncio.sleep(0.75) + continue + logger.warning( + "[%s] Direct auto-thread creation failed and reconciliation remained " + "inconclusive; skipping fallback to avoid a duplicate thread. Direct error: %s", + self.name, + direct_error, + ) + return None try: seed_msg = await message.channel.send( diff --git a/tests/gateway/test_discord_channel_controls.py b/tests/gateway/test_discord_channel_controls.py index 009031f8b8c9..03a210360b0a 100644 --- a/tests/gateway/test_discord_channel_controls.py +++ b/tests/gateway/test_discord_channel_controls.py @@ -61,7 +61,6 @@ def __init__(self, channel_id: int = 1, name: str = "general", guild_name: str = self.name = name self.guild = SimpleNamespace(name=guild_name) self.topic = None - self.send = AsyncMock() class FakeThread: @@ -240,3 +239,4 @@ def test_config_bridges_ignored_channels(monkeypatch, tmp_path): import os assert os.getenv("DISCORD_IGNORED_CHANNELS") == "111,222" + diff --git a/tests/gateway/test_discord_slash_commands.py b/tests/gateway/test_discord_slash_commands.py index e3e5a39ff57c..a32659b234cc 100644 --- a/tests/gateway/test_discord_slash_commands.py +++ b/tests/gateway/test_discord_slash_commands.py @@ -103,7 +103,7 @@ def adapter(): adapter._client = SimpleNamespace( tree=FakeTree(), get_channel=lambda _id: None, - fetch_channel=AsyncMock(), + fetch_channel=AsyncMock(return_value=None), user=SimpleNamespace(id=99999, name="HermesBot"), ) adapter._text_batch_delay_seconds = 0 # disable batching for tests @@ -427,6 +427,110 @@ async def test_auto_create_thread_strips_mention_syntax_from_name(adapter): assert name == "please help" +def _failed_auto_thread_message(): + return SimpleNamespace( + id=123, + content="Trip idea", + create_thread=AsyncMock(side_effect=ConnectionError("response lost")), + thread=None, + channel=SimpleNamespace(send=AsyncMock()), + author=SimpleNamespace(display_name="Jezza"), + ) + + +@pytest.mark.asyncio +async def test_auto_create_thread_reuses_attached_starter_thread(adapter): + existing = _FakeThreadChannel(channel_id=123, name="already-there") + message = _failed_auto_thread_message() + message.thread = existing + + result = await adapter._auto_create_thread(message) + + assert result is existing + message.channel.send.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_auto_create_thread_reuses_cached_starter_thread(adapter): + existing = _FakeThreadChannel(channel_id=123, name="already-there") + message = _failed_auto_thread_message() + adapter._client.get_channel = MagicMock(return_value=existing) + + result = await adapter._auto_create_thread(message) + + assert result is existing + adapter._client.get_channel.assert_called_once_with(message.id) + adapter._client.fetch_channel.assert_not_awaited() + message.channel.send.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_auto_create_thread_fetches_starter_thread_after_cache_miss(adapter): + existing = _FakeThreadChannel(channel_id=123, name="already-there") + message = _failed_auto_thread_message() + adapter._client.get_channel = MagicMock(return_value=None) + adapter._client.fetch_channel = AsyncMock(return_value=existing) + + result = await adapter._auto_create_thread(message) + + assert result is existing + adapter._client.fetch_channel.assert_awaited_once_with(message.id) + message.channel.send.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_auto_create_thread_retries_reconciliation_before_fallback(adapter, monkeypatch): + existing = _FakeThreadChannel(channel_id=123, name="eventually-visible") + message = _failed_auto_thread_message() + adapter._client.get_channel = MagicMock(return_value=None) + adapter._client.fetch_channel = AsyncMock(side_effect=[None, existing]) + sleep = AsyncMock() + monkeypatch.setattr("plugins.platforms.discord.adapter.asyncio.sleep", sleep) + + result = await adapter._auto_create_thread(message) + + assert result is existing + assert adapter._client.fetch_channel.await_count == 2 + sleep.assert_awaited_once_with(0.25) + message.channel.send.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_auto_create_thread_does_not_reuse_unrelated_channel(adapter, monkeypatch): + unrelated = _FakeTextChannel(channel_id=999) + fallback = SimpleNamespace(id=456, name="fallback") + seed_message = SimpleNamespace(create_thread=AsyncMock(return_value=fallback)) + message = _failed_auto_thread_message() + message.thread = unrelated + message.channel.send.return_value = seed_message + adapter._client.get_channel = MagicMock(return_value=unrelated) + adapter._client.fetch_channel = AsyncMock(return_value=None) + monkeypatch.setattr("plugins.platforms.discord.adapter.asyncio.sleep", AsyncMock()) + + result = await adapter._auto_create_thread(message) + + assert result is fallback + message.channel.send.assert_awaited_once() + + +@pytest.mark.asyncio +async def test_auto_create_thread_skips_fallback_when_reconciliation_is_inconclusive( + adapter, + monkeypatch, +): + message = _failed_auto_thread_message() + adapter._client.get_channel = MagicMock(return_value=None) + adapter._client.fetch_channel = AsyncMock(side_effect=ConnectionError("lookup failed")) + monkeypatch.setattr("plugins.platforms.discord.adapter.asyncio.sleep", AsyncMock()) + + result = await adapter._auto_create_thread(message) + + assert result is None + assert message.create_thread.await_count == 2 + assert adapter._client.fetch_channel.await_count == 4 + message.channel.send.assert_not_awaited() + + @pytest.mark.asyncio async def test_rename_thread_edits_only_when_current_name_matches(adapter): thread = SimpleNamespace( @@ -601,4 +705,3 @@ def test_register_skill_command_payload_fits_discord_8kb_limit(adapter): f"point of this design is that it stays small regardless of skill count" ) -