hermes - 💡(How to fix) Fix refactor(gateway): migrate Discord adapter to bundled plugin (first migration of an existing built-in)

Official PRs (…)
ON THIS PAGE

Recommended Tools

×6

Utilities matched from this issue’s tags and category — try them while you read without losing context.

GitHub issue graph ai analysis

Paste a GitHub issue URL. We fetch that issue, discover linked issues from bodies/comments/timeline, collect linked pull requests, and produce a structured English report.

The report is written in English Markdown for sharing and archival.

Helpful · Quick feedback

Loading…

Root Cause

  1. Is the sys.modules aliasing shim acceptable as the migration pattern? It's an unusual Python pattern, but it's the difference between a ~100-LOC PR and a ~700-LOC PR. If unacceptable, the alternative is rewriting all 49 import/patch sites, which I'm willing to do but want a thumbs-up first.
  2. Should the shim stay forever, or be removed in a follow-up? Argument for keeping: protects out-of-tree consumers (third-party plugins, downstream forks, scripts). Argument for removing: marginal — 20 LOC.
  3. Should the generic gateway_runner injection be its own preceding PR? Splitting it out is the safer / more incremental option (it benefits webhook + future plugin platforms regardless of whether Discord migrates). Folding it in keeps the Discord PR self-evidencing — the move needs the generic hook because the discord-specific elif disappears.

Fix Action

Fix / Workaround

  • 5,101 LOC in gateway/platforms/discord.py (larger than google_chat at ~3.3k)
  • 25 dedicated test files (tests/gateway/test_discord_*.py) covering slash commands, voice mode, role-based DM auth, free-response channels, channel skill bindings, model picker, attachments, reactions, threads, etc.
  • ~1,976 Platform.DISCORD references outside the adapter
  • 28 sites import from gateway.platforms.discord import … (1 in tools/send_message_tool.py, 27 in test files)
  • 21 mock.patch("gateway.platforms.discord.X") test sites

Aliasing both import paths to the same module object means:

  • All 28 from gateway.platforms.discord import X sites work unchanged (including underscored private names like _component_check_auth and _DISCORD_COMMAND_SYNC_STATE_SUBDIR)
  • All 21 mock.patch("gateway.platforms.discord.X", ...) test sites patch the real attribute the running code reads
  • Module-level globals (DISCORD_AVAILABLE, discord, DiscordMessage, Intents, commands) stay in sync after check_discord_requirements() rebinds them via global declarations

Without this trick, the migration would require touching all 49 import/patch sites (28 imports + 21 patch strings), which is the kind of "extra LOC without context" change the project already pushes back on.

Code Example

import sys as _sys
from plugins.platforms.discord import adapter as _adapter
_sys.modules[__name__] = _adapter
RAW_BUFFERClick to expand / collapse

Migrate Discord platform adapter to the bundled plugin system

Background

Issue #3823 proposes making platform adapters self-registering to eliminate the per-platform boilerplate tax. Since #3823 was filed, four platform adapters have been added under plugins/platforms/:

PluginStatusFirst commit
irc✅ Reference impl8f144fe36 (the original "pluggable platform adapter registry" PR)
teams✅ Plugin from day 1b3137d758
google_chat✅ Plugin from day 144cd79e79
line✅ Plugin from day 1#23197 (50f9fee98)

All four were born as plugins. None of the existing built-in platform adapters in gateway/platforms/ has been migrated to the new system yet — so the registry pattern hasn't been validated against an existing, deeply-integrated platform.

This issue proposes Discord as the first migration target.

Why Discord first

Discord is the largest and most deeply-integrated built-in platform adapter:

  • 5,101 LOC in gateway/platforms/discord.py (larger than google_chat at ~3.3k)
  • 25 dedicated test files (tests/gateway/test_discord_*.py) covering slash commands, voice mode, role-based DM auth, free-response channels, channel skill bindings, model picker, attachments, reactions, threads, etc.
  • ~1,976 Platform.DISCORD references outside the adapter
  • 28 sites import from gateway.platforms.discord import … (1 in tools/send_message_tool.py, 27 in test files)
  • 21 mock.patch("gateway.platforms.discord.X") test sites

If the migration pattern works for Discord, it works for everything else.

Spike result

I prototyped the move in a worktree (branch spike/discord-plugin-extraction). Approach:

  1. git mv gateway/platforms/discord.py → plugins/platforms/discord/adapter.py
  2. Add plugins/platforms/discord/{__init__.py, plugin.yaml} (37 LOC)
  3. Append a register(ctx) block to adapter.py (37 LOC) — the existing PlatformEntry dataclass already supports every Discord-relevant field (allowed_users_env, allow_all_env, cron_deliver_env_var, max_message_length, emoji, is_connected, required_env, install_hint)
  4. Add a 20-line sys.modules-aliasing shim at gateway/platforms/discord.py so the legacy import path resolves to the same Python module object as the new canonical path
  5. Replace the discord-specific elif in _create_adapter() with a generic post-creation hook: any plugin adapter that declares a gateway_runner attribute gets it auto-injected (also subsumes the existing webhook-specific elif branch) — 6 LOC
  6. 3-line fix to tests/gateway/test_discord_imports.py (the only test that purges gateway.platforms.discord from sys.modules — it now also needs to purge the canonical name)

Net real new code: ~91 lines plus the file move (which GitHub renders as a rename, not a real diff).

Test results

  • 569/569 dedicated Discord/voice/send tests pass on the spike
  • Broader gateway/cron/integration sweep: 6 pre-existing failures on main, 0 new failures introduced by the spike (verified by running the same selectors on main for comparison)

The load-bearing trick

The 20-line shim at gateway/platforms/discord.py:

import sys as _sys
from plugins.platforms.discord import adapter as _adapter
_sys.modules[__name__] = _adapter

Aliasing both import paths to the same module object means:

  • All 28 from gateway.platforms.discord import X sites work unchanged (including underscored private names like _component_check_auth and _DISCORD_COMMAND_SYNC_STATE_SUBDIR)
  • All 21 mock.patch("gateway.platforms.discord.X", ...) test sites patch the real attribute the running code reads
  • Module-level globals (DISCORD_AVAILABLE, discord, DiscordMessage, Intents, commands) stay in sync after check_discord_requirements() rebinds them via global declarations

Without this trick, the migration would require touching all 49 import/patch sites (28 imports + 21 patch strings), which is the kind of "extra LOC without context" change the project already pushes back on.

Scope

In scope for the migration PR

  • File move + plugin shell + register() block
  • sys.modules shim
  • Generic gateway_runner injection (replaces both the discord-specific and webhook-specific elif branches in _create_adapter)
  • 3-line test fix for test_discord_imports.py

Explicitly out of scope (separate follow-ups, only after this lands)

  • gateway/config.py Discord config-key normalization (lines ~851–905): runs before any adapter exists. Could later move into env_enablement_fn (the same hook teams uses) but isn't blocking.
  • tools/send_message_tool.py::_send_discord: the standalone HTTP send used when cron runs out-of-process. Could later become a standalone_sender_fn like teams has, but works as-is.
  • tools/discord_tool.py and discord_admin toolset: user-facing tools, independent of the gateway adapter.
  • The Platform.DISCORD enum literal: a stable identifier used as dict keys throughout the codebase. Removing it is a separate, much larger refactor and provides no real benefit.

Open questions for maintainers

  1. Is the sys.modules aliasing shim acceptable as the migration pattern? It's an unusual Python pattern, but it's the difference between a ~100-LOC PR and a ~700-LOC PR. If unacceptable, the alternative is rewriting all 49 import/patch sites, which I'm willing to do but want a thumbs-up first.
  2. Should the shim stay forever, or be removed in a follow-up? Argument for keeping: protects out-of-tree consumers (third-party plugins, downstream forks, scripts). Argument for removing: marginal — 20 LOC.
  3. Should the generic gateway_runner injection be its own preceding PR? Splitting it out is the safer / more incremental option (it benefits webhook + future plugin platforms regardless of whether Discord migrates). Folding it in keeps the Discord PR self-evidencing — the move needs the generic hook because the discord-specific elif disappears.

PR ready when this lands

I'd like to open a PR with the spike's contents once the approach is acked. cc @teknium1 since this concerns issue #3823 directly.

Vote matrix · Quick signals

Works
Did the solution work? Tap to confirm.
Easy Fix
Was it a quick fix?
Time Saver
Did it save you time?
Blocking
Was it severely blocking?
Common Issue
Are others likely hitting this too?
Flaky / Intermittent
Is it intermittent?
Verified / Reproducible
Can you reproduce it reliably?
Loading…

Still need to ship something?

×6

Another batch ranked right after the header list — different links, same matching logic.

Back to top recommendations

TRENDING

hermes - 💡(How to fix) Fix refactor(gateway): migrate Discord adapter to bundled plugin (first migration of an existing built-in)