diff --git a/plugins/platforms/discord/adapter.py b/plugins/platforms/discord/adapter.py index d433bcb4e..241a670c1 100644 --- a/plugins/platforms/discord/adapter.py +++ b/plugins/platforms/discord/adapter.py @@ -5020,7 +5020,11 @@ class DiscordAdapter(BasePlatformAdapter): async def _auto_create_thread(self, message: 'DiscordMessage') -> Optional[Any]: """Create a thread from a user message for auto-threading. - Returns the created thread object, or ``None`` on failure. + Returns the created thread object, or ``None`` on failure. Both the + primary ``message.create_thread`` and the seed-message fallback are + retried once after a short backoff so transient connect errors + (e.g. ``Cannot connect to host discord.com:443``) don't immediately + burn through to the caller's failure path (#20243). """ # Build a short thread name from the message. Strip Discord mention # syntax (users / roles / channels) so thread titles don't end up @@ -5035,28 +5039,44 @@ class DiscordAdapter(BasePlatformAdapter): if len(content) > 80: thread_name = thread_name[:77] + "..." - try: - thread = await message.create_thread(name=thread_name, auto_archive_duration=1440) - return thread - except Exception as direct_error: - display_name = getattr(getattr(message, "author", None), "display_name", None) or "unknown user" - reason = f"Auto-threaded from mention by {display_name}" + display_name = getattr(getattr(message, "author", None), "display_name", None) or "unknown user" + reason = f"Auto-threaded from mention by {display_name}" + + last_direct_error: Exception | None = None + last_fallback_error: Exception | None = None + + for attempt in range(2): try: - seed_msg = await message.channel.send(f"\U0001f9f5 Thread created by Hermes: **{thread_name}**") - thread = await seed_msg.create_thread( - name=thread_name, - auto_archive_duration=1440, - reason=reason, - ) + thread = await message.create_thread(name=thread_name, auto_archive_duration=1440) return thread - except Exception as fallback_error: - logger.warning( - "[%s] Auto-thread creation failed. Direct error: %s. Fallback error: %s", - self.name, - direct_error, - fallback_error, - ) - return None + except Exception as direct_error: + last_direct_error = direct_error + try: + seed_msg = await message.channel.send( + f"\U0001f9f5 Thread created by Hermes: **{thread_name}**" + ) + thread = await seed_msg.create_thread( + name=thread_name, + auto_archive_duration=1440, + reason=reason, + ) + return thread + except Exception as fallback_error: + last_fallback_error = fallback_error + if attempt == 0: + # Brief backoff before the second attempt — most failures + # in this path are transient connect errors that recover + # within a second or two. + await asyncio.sleep(0.75) + continue + + logger.warning( + "[%s] Auto-thread creation failed after retry. Direct error: %s. Fallback error: %s", + self.name, + last_direct_error, + last_fallback_error, + ) + return None async def create_handoff_thread( self, @@ -5742,6 +5762,26 @@ class DiscordAdapter(BasePlatformAdapter): # event is dropped before it can trigger a second agent run. # Fixes #51057. self._dedup.is_duplicate(str(thread.id)) + else: + # Auto-threading is the configured routing target for this + # message; if it fails we must NOT silently fall back to an + # inline parent-channel reply (#20243). That breaks + # thread-first Discord workflows by dumping a new task into + # a shared channel. Surface a short visible error so the + # user can retry once Discord recovers, and skip agent + # invocation for this message. + try: + await message.channel.send( + "⚠️ Hermes could not create a Discord thread for " + "this message, so the request was not processed. Please retry." + ) + except Exception as notify_error: + logger.warning( + "[%s] Failed to notify user of auto-thread failure: %s", + self.name, + notify_error, + ) + return referenced_attachments = [] reference = getattr(message, "reference", None) diff --git a/tests/gateway/test_discord_channel_controls.py b/tests/gateway/test_discord_channel_controls.py index 3142ef839..d84d56fbb 100644 --- a/tests/gateway/test_discord_channel_controls.py +++ b/tests/gateway/test_discord_channel_controls.py @@ -140,6 +140,11 @@ async def test_non_ignored_channel_processes_normally(adapter, monkeypatch): monkeypatch.setenv("DISCORD_IGNORED_CHANNELS", "500,600") monkeypatch.delenv("DISCORD_FREE_RESPONSE_CHANNELS", raising=False) + # Stub auto-thread creation so this test focuses on ignored-channel + # routing only — auto-thread failures now correctly skip agent invocation + # (#20243), which would otherwise mask the assertion below. + adapter._auto_create_thread = AsyncMock(return_value=FakeThread(channel_id=999)) + message = make_message(channel=FakeTextChannel(channel_id=700), content="hello") await adapter._handle_message(message) @@ -167,6 +172,11 @@ async def test_ignored_channels_empty_string_ignores_nothing(adapter, monkeypatc monkeypatch.setenv("DISCORD_IGNORED_CHANNELS", "") monkeypatch.delenv("DISCORD_FREE_RESPONSE_CHANNELS", raising=False) + # Stub auto-thread creation so this test focuses on ignored-channel + # routing only — auto-thread failures now correctly skip agent invocation + # (#20243), which would otherwise mask the assertion below. + adapter._auto_create_thread = AsyncMock(return_value=FakeThread(channel_id=999)) + message = make_message(channel=FakeTextChannel(channel_id=500), content="hello") await adapter._handle_message(message) @@ -281,6 +291,71 @@ async def test_no_thread_with_auto_thread_disabled_is_noop(adapter, monkeypatch) adapter.handle_message.assert_awaited_once() +# ── auto-thread failure must not silently fall back to inline (#20243) ── + + +@pytest.mark.asyncio +async def test_auto_thread_failure_skips_agent_and_notifies_user(adapter, monkeypatch): + """Auto-thread creation failure must not trigger an inline parent-channel reply. + + Before #20243, ``effective_channel = auto_threaded_channel or message.channel`` + silently routed the response back to the parent channel when thread creation + failed, breaking thread-first Discord workflows. The fix surfaces a short + visible error to the parent channel and skips agent invocation entirely so + the user can retry. + """ + monkeypatch.setenv("DISCORD_REQUIRE_MENTION", "false") + monkeypatch.setenv("DISCORD_AUTO_THREAD", "true") + monkeypatch.delenv("DISCORD_NO_THREAD_CHANNELS", raising=False) + monkeypatch.delenv("DISCORD_IGNORED_CHANNELS", raising=False) + monkeypatch.delenv("DISCORD_FREE_RESPONSE_CHANNELS", raising=False) + + adapter._auto_create_thread = AsyncMock(return_value=None) + + channel = FakeTextChannel(channel_id=800) + channel.send = AsyncMock() + message = make_message(channel=channel, content="hello") + await adapter._handle_message(message) + + adapter._auto_create_thread.assert_awaited_once() + # Agent must NOT be invoked when the routing target failed. + adapter.handle_message.assert_not_awaited() + # User gets a visible explanation in the parent channel instead of a silent + # inline reply. + channel.send.assert_awaited_once() + sent_text = channel.send.await_args.args[0] + assert "could not create" in sent_text.lower() + assert "thread" in sent_text.lower() + + +@pytest.mark.asyncio +async def test_auto_thread_failure_notify_error_does_not_crash(adapter, monkeypatch): + """If even the failure-notification send raises, we still skip the agent. + + ``message.channel.send`` itself can fail (the same connect issue that + killed thread creation often kills plain sends too). The handler should + swallow the secondary error and still avoid invoking the agent. + """ + monkeypatch.setenv("DISCORD_REQUIRE_MENTION", "false") + monkeypatch.setenv("DISCORD_AUTO_THREAD", "true") + monkeypatch.delenv("DISCORD_NO_THREAD_CHANNELS", raising=False) + monkeypatch.delenv("DISCORD_IGNORED_CHANNELS", raising=False) + monkeypatch.delenv("DISCORD_FREE_RESPONSE_CHANNELS", raising=False) + + adapter._auto_create_thread = AsyncMock(return_value=None) + + channel = FakeTextChannel(channel_id=800) + channel.send = AsyncMock(side_effect=RuntimeError("Cannot connect to host discord.com:443")) + message = make_message(channel=channel, content="hello") + + # No exception must propagate. + await adapter._handle_message(message) + + adapter._auto_create_thread.assert_awaited_once() + adapter.handle_message.assert_not_awaited() + channel.send.assert_awaited_once() + + # ── config.py bridging ─────────────────────────────────────────────── diff --git a/tests/gateway/test_discord_free_response.py b/tests/gateway/test_discord_free_response.py index 589c8a7c5..3ed20c2fb 100644 --- a/tests/gateway/test_discord_free_response.py +++ b/tests/gateway/test_discord_free_response.py @@ -202,6 +202,10 @@ async def test_discord_defaults_to_require_mention(adapter, monkeypatch): async def test_discord_free_response_in_server_channels(adapter, monkeypatch): monkeypatch.setenv("DISCORD_REQUIRE_MENTION", "false") monkeypatch.delenv("DISCORD_FREE_RESPONSE_CHANNELS", raising=False) + # Auto-thread failures now correctly skip agent invocation (#20243), and + # FakeTextChannel has no real ``create_thread``. Disable auto-thread so the + # routing assertion below stays focused on free-response gating. + monkeypatch.setenv("DISCORD_AUTO_THREAD", "false") message = make_message(channel=FakeTextChannel(channel_id=123), content="hello from channel") @@ -334,6 +338,10 @@ async def test_discord_forum_parent_in_free_response_list_allows_forum_thread(ad async def test_discord_accepts_and_strips_bot_mentions_when_required(adapter, monkeypatch): monkeypatch.setenv("DISCORD_REQUIRE_MENTION", "true") monkeypatch.delenv("DISCORD_FREE_RESPONSE_CHANNELS", raising=False) + # Auto-thread failures now correctly skip agent invocation (#20243). + # FakeTextChannel can't satisfy the real ``create_thread`` API, so disable + # auto-thread to keep this test focused on mention-strip behaviour. + monkeypatch.setenv("DISCORD_AUTO_THREAD", "false") bot_user = adapter._client.user message = make_message(