From 9c311038fa215127c6be9cc31bfdb91bb5ecd1a7 Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Mon, 14 Sep 2026 21:08:09 -0700 Subject: [PATCH] fix(discord): slash registration and /skill refresh scan the catalog off the event loop connect() called _register_slash_commands inline on every (re)connect, and /reload-skills called refresh_skill_group inline; both run discord_skill_commands_by_category, the same per-skill path-resolution walk the Telegram menu paid in #110707. On a 1.5k-skill install that holds the loop past the liveness watchdog. Registration now hops through asyncio.to_thread from connect(); refresh_skill_group is a coroutine that hops the rescan the same way (the reload handler already awaits an awaitable result). Contextvar-scoped profile overrides propagate through to_thread. One invariant test: the loop keeps ticking while the scan blocks, from both sites. Red on the PR head, green here. Review finding: Discord _register_slash_commands/refresh_skill_group ran the skill catalog disk scan synchronously on the event loop. --- plugins/platforms/discord/adapter.py | 7 ++-- tests/gateway/test_discord_connect.py | 35 +++++++++++++++++++ .../test_reload_skills_discord_resync.py | 3 +- 3 files changed, 41 insertions(+), 4 deletions(-) diff --git a/plugins/platforms/discord/adapter.py b/plugins/platforms/discord/adapter.py index 60e8f017c0..598fadc13e 100644 --- a/plugins/platforms/discord/adapter.py +++ b/plugins/platforms/discord/adapter.py @@ -1346,7 +1346,8 @@ class DiscordAdapter(DiscordMediaMixin, BasePlatformAdapter): guild_id, ) if self._slash_commands: - self._register_slash_commands() + # Registration walks the skill catalog on disk (#110707); keep the loop free. + await asyncio.to_thread(self._register_slash_commands) self._disconnecting = False self._bot_task = asyncio.create_task(self._client.start(self.config.token)) self._bot_task.add_done_callback(self._handle_bot_task_done) @@ -4486,11 +4487,11 @@ class DiscordAdapter(DiscordMediaMixin, BasePlatformAdapter): self._skill_lookup = {n: (d, k) for n, d, k in entries} self._skill_group_hidden_count = hidden - def refresh_skill_group(self) -> tuple[int, int]: + async def refresh_skill_group(self) -> tuple[int, int]: """Rescan skills and refresh live ``/skill`` autocomplete; returns ``(new_count, hidden_count)``. Called after ``reload_skills``; no ``tree.sync()`` since autocomplete options are dynamic.""" try: - self._refresh_skill_catalog_state() + await asyncio.to_thread(self._refresh_skill_catalog_state) except Exception as exc: logger.warning( "[%s] Failed to refresh /skill autocomplete after reload: %s", self.name, exc, diff --git a/tests/gateway/test_discord_connect.py b/tests/gateway/test_discord_connect.py index 401ed5ddfb..17ed9de560 100644 --- a/tests/gateway/test_discord_connect.py +++ b/tests/gateway/test_discord_connect.py @@ -752,3 +752,38 @@ class TestPrivilegedIntentsRequiredFatal: assert "discord.com/developers/applications" in (adapter.fatal_error_message or "") assert adapter._bot_task is None + + +@pytest.mark.asyncio +async def test_skill_catalog_scan_runs_off_the_event_loop(monkeypatch): + """A slow skill scan (#110707) must not block the gateway loop from either Discord site: + slash registration inside connect() (every reconnect) and /reload-skills' refresh_skill_group.""" + import threading + + adapter = DiscordAdapter(PlatformConfig(enabled=True, token="test-token")) + monkeypatch.setattr("gateway.status.acquire_scoped_lock", lambda scope, identity, metadata=None: (True, None)) + monkeypatch.setattr("gateway.status.release_scoped_lock", lambda scope, identity: None) + monkeypatch.setattr(discord_platform.Intents, "default", lambda: SimpleNamespace( + message_content=False, dm_messages=False, guild_messages=False, members=False, voice_states=False)) + monkeypatch.setattr(discord_platform.commands, "Bot", lambda **kw: FakeBot(intents=kw["intents"])) + monkeypatch.setattr(adapter, "_resolve_allowed_usernames", AsyncMock()) + + async def _scan_leaves_loop_free(run_site): + scan_started, loop_ticked = threading.Event(), threading.Event() + + def _blocking_scan(*, reserved_names): + scan_started.set() + # Only a free loop can set loop_ticked while this wait is in progress. + return ({}, [("x", "desc", "/x")], 0) if loop_ticked.wait(timeout=1) else ({}, [], 0) + + monkeypatch.setattr("hermes_cli.commands_platforms.discord_skill_commands_by_category", _blocking_scan) + task = asyncio.create_task(run_site()) + await asyncio.to_thread(scan_started.wait, 1) + loop_ticked.set() + await task + return [n for n, _d, _k in adapter._skill_entries] == ["x"] + + assert await _scan_leaves_loop_free(adapter.connect) + adapter._skill_entries = [] + assert await _scan_leaves_loop_free(adapter.refresh_skill_group) + await adapter.disconnect() diff --git a/tests/gateway/test_reload_skills_discord_resync.py b/tests/gateway/test_reload_skills_discord_resync.py index e14562ce6b..cfd32946c1 100644 --- a/tests/gateway/test_reload_skills_discord_resync.py +++ b/tests/gateway/test_reload_skills_discord_resync.py @@ -22,6 +22,7 @@ data the live callbacks already read from. """ from __future__ import annotations +import asyncio from unittest.mock import MagicMock @@ -73,7 +74,7 @@ class TestRefreshSkillGroup: fake_collector, ) - new_count, hidden = adapter.refresh_skill_group() + new_count, hidden = asyncio.run(adapter.refresh_skill_group()) assert new_count == 1 assert hidden == 0