Skip to content

chore(gateway): drop plugin-migrated platforms from /update allowlist#32525

Closed
kshitijk4poor wants to merge 1 commit into
NousResearch:mainfrom
kshitijk4poor:chore/drop-migrated-platforms-from-update-allowlist
Closed

chore(gateway): drop plugin-migrated platforms from /update allowlist#32525
kshitijk4poor wants to merge 1 commit into
NousResearch:mainfrom
kshitijk4poor:chore/drop-migrated-platforms-from-update-allowlist

Conversation

@kshitijk4poor

Copy link
Copy Markdown
Collaborator

Summary

Drops Platform.DISCORD and Platform.MATTERMOST from the hardcoded _UPDATE_ALLOWED_PLATFORMS frozenset in gateway/run.py. Both platforms are bundled plugins now (PR #24356 migrated Discord, PR #31748 migrated Mattermost) and they declare allow_update_command=True on their PlatformEntry. The gate at _handle_update_command already falls back to the registry when a platform isn't in the frozenset:

_allowed = self._UPDATE_ALLOWED_PLATFORMS
if platform not in _allowed:
    entry = platform_registry.get(platform.value)
    if not entry or not entry.allow_update_command:
        return t("gateway.update.platform_not_messaging")

Today both entries say "allowed" twice — once in the frozenset, once on the registry. Cleaning up the frozenset makes the registry flag actually do work for these platforms instead of being a redundant no-op.

What's NOT touched

  • The Home Assistant entry stays — PR refactor(gateway): migrate Home Assistant adapter to bundled plugin #32500 migrates HA but isn't merged yet. Drop it in a follow-up after that lands.
  • Other plugin-only platforms (teams, irc, line, google_chat, ntfy, simplex) were never in the frozenset to begin with — they've always relied on the registry fallback.
  • Remaining built-ins (telegram, slack, whatsapp, signal, matrix, email, sms, dingtalk, feishu, wecom, weixin, bluebubbles, qqbot, local) stay in the frozenset; they're not plugin-migrated yet.

Verification

# All currently-migrated plugin platforms declare allow_update_command=True:
from hermes_cli.plugins import PluginManager
PluginManager().discover_and_load(force=True)
from gateway.platform_registry import platform_registry
for n in ['discord', 'mattermost', 'teams', 'irc', 'line', 'google_chat', 'ntfy', 'simplex']:
    e = platform_registry.get(n)
    print(n, e.allow_update_command)
# discord True / mattermost True / teams True / irc True / line True / google_chat True / ntfy True / simplex True

Empirical test from the worktree:

# /update on Discord/Mattermost still works (passes the gate via registry fallback)
# /update on Webhook/API_Server still blocked (not messaging platforms)

Tests

Added TestUpdateCommandPlatformGate to tests/gateway/test_update_command.py with five cases that pin down every branch of the allowlist gate. The gate previously had zero direct coverage:

  • test_blocks_programmatic_interfacePlatform.WEBHOOK must be blocked
  • test_blocks_api_server_platformPlatform.API_SERVER must be blocked
  • test_allows_plugin_platform_via_registry_fallbackPlatform.DISCORD must pass via registry fallback (the regression test for this PR's change)
  • test_allows_mattermost_via_registry_fallback — same for Platform.MATTERMOST
  • test_allows_builtin_platform_in_allowlistPlatform.TELEGRAM must still pass directly via the frozenset

Each plugin-platform test asserts the precondition Platform.X not in _UPDATE_ALLOWED_PLATFORMS explicitly, so if anyone re-adds Discord/Mattermost to the frozenset in the future, the tests fail loudly.

All 74 /update-related tests pass (was 69 before, +5 new).

Surfaced during

Self-review of PR #32500 (Home Assistant migration). The migration playbook calls out this redundancy as out-of-scope tech debt for the migration PR itself; this PR closes that follow-up for the platforms already migrated.

`gateway/run.py::_UPDATE_ALLOWED_PLATFORMS` was a hardcoded frozenset
listing every messaging platform allowed to invoke the `/update` slash
command.  Plugin-migrated platforms (currently Discord and Mattermost,
soon also Home Assistant via NousResearch#32500) declare `allow_update_command=True`
on their `PlatformEntry`, and `_handle_update_command` already falls
back to the registry when a platform isn't in the frozenset.  The result
was a silent redundancy: those entries said "allowed" twice, and the
registry flag was a no-op for them in practice.

  - Removed `Platform.DISCORD` and `Platform.MATTERMOST` from the frozenset.
  - Updated the docstring to make the split explicit (built-ins live in
    the frozenset; plugins use `allow_update_command` on the registry entry).

The remaining frozenset entries are all still built-in platforms living
under `gateway/platforms/` today.  Future plugin migrations should drop
their entry from the frozenset as part of the migration PR (or in a
sibling chore PR like this one).

Added a `TestUpdateCommandPlatformGate` test class that pins down all
three branches of the gate so future changes don't silently regress:

  - Programmatic interfaces (`Platform.WEBHOOK`, `Platform.API_SERVER`)
    must remain blocked.
  - Plugin-migrated platforms (Discord, Mattermost) must pass via the
    registry fallback.
  - Built-in platforms in the hardcoded frozenset (Telegram) must
    still pass without needing the registry.

The gate previously had zero direct test coverage — its only existing
coverage was `test_no_adapter_for_platform` which exercised a different
code path.
@alt-glitch alt-glitch added type/refactor Code restructuring, no behavior change P3 Low — cosmetic, nice to have comp/gateway Gateway runner, session dispatch, delivery comp/plugins Plugin system and bundled plugins labels May 26, 2026
@teknium1

teknium1 commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

Merged via #40711 — your commit was cherry-picked onto current main with your authorship preserved in git log (the original branch was 1211 commits behind). The CI flake on the rerun was an unrelated concurrent-tool-execution shard race, not your diff. Thanks for the cleanup and the +5 gate-coverage tests.

@teknium1 teknium1 closed this Jun 6, 2026
kshitijk4poor added a commit to kshitijk4poor/hermes-agent that referenced this pull request Jun 7, 2026
signal is now a bundled plugin; its PlatformEntry already permits /update via
allow_update_command (defaults True), honored by the registry fallback in
_handle_update_command. The hardcoded Platform.SIGNAL entry in
_UPDATE_ALLOWED_PLATFORMS is therefore redundant. Same cleanup as NousResearch#32525
did for Discord/Mattermost.
kshitijk4poor added a commit to kshitijk4poor/hermes-agent that referenced this pull request Jun 7, 2026
qqbot is now a bundled plugin; its PlatformEntry already permits /update via
allow_update_command (defaults True), honored by the registry fallback in
_handle_update_command. The hardcoded Platform.QQBOT entry in
_UPDATE_ALLOWED_PLATFORMS is therefore redundant. Same cleanup as NousResearch#32525
did for Discord/Mattermost.
kshitijk4poor added a commit to kshitijk4poor/hermes-agent that referenced this pull request Jun 7, 2026
weixin is now a bundled plugin; its PlatformEntry already permits /update via
allow_update_command (defaults True), honored by the registry fallback in
_handle_update_command. The hardcoded Platform.WEIXIN entry in
_UPDATE_ALLOWED_PLATFORMS is therefore redundant. Same cleanup as NousResearch#32525
did for Discord/Mattermost.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have type/refactor Code restructuring, no behavior change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants