Skip to content

fix(gateway): honor gateway_restart_notification under top-level platform sections (#24644)#24663

Closed
0xsir0000 wants to merge 1 commit into
NousResearch:mainfrom
0xsir0000:fix/gateway-restart-notification-bridge
Closed

fix(gateway): honor gateway_restart_notification under top-level platform sections (#24644)#24663
0xsir0000 wants to merge 1 commit into
NousResearch:mainfrom
0xsir0000:fix/gateway-restart-notification-bridge

Conversation

@0xsir0000

Copy link
Copy Markdown
Contributor

Summary

PlatformConfig.gateway_restart_notification has been a no-op when set under the user-friendly top-level platform section (e.g. telegram:) in config.yaml. The bridge loop in load_gateway_config (gateway/config.py:758) only forwarded a fixed allowlist of keys, and gateway_restart_notification wasn't on the list, so:

  1. telegram.gateway_restart_notification: false never reached the platforms data dict.
  2. PlatformConfig.from_dict() read the safe default (True) every time.
  3. The suppression check at gateway/run.py:2767 always evaluated to truthy → shutdown/restart notifications were always sent.

Fix

The issue's suggested fix (route the key through bridged) is not quite right — extra.update(bridged) would land it in extra, where PlatformConfig.from_dict() never looks (it reads data.get(\"gateway_restart_notification\") from the top level, see gateway/config.py:313). enabled already solves the same problem by writing directly to plat_data at the top level. This PR mirrors that path:

restart_notify_was_explicit = (
    \"gateway_restart_notification\" in platform_cfg
)
if not bridged and not enabled_was_explicit and not restart_notify_was_explicit:
    continue
...
if restart_notify_was_explicit:
    plat_data[\"gateway_restart_notification\"] = platform_cfg[
        \"gateway_restart_notification\"
    ]

Including the key in the early-continue guard matters: without it, sections that only set gateway_restart_notification (no other bridgeable keys, no explicit enabled) would fall through to continue and never even materialise an entry in platforms_data.

Test plan

  • test_bridges_gateway_restart_notification_from_telegram_sectiontelegram.gateway_restart_notification: false round-trips to False (the reported case).
  • test_bridges_gateway_restart_notification_true_explicit — explicit true under discord: also bridges (proves the bridge isn't accidentally swallowing values that match the default).
  • test_gateway_restart_notification_defaults_true_when_unset — omitting the key keeps True so existing configs are unaffected.
  • All 43 existing tests in tests/gateway/test_config.py still pass.

Without the fix, the two assertions targeting Platform.DISCORD / Platform.TELEGRAM fail with KeyError because the platform entry isn't even created (verified by reverting the fix locally).

Fixes #24644

…form sections (NousResearch#24644)

PlatformConfig.gateway_restart_notification has been a no-op when set
under the user-friendly top-level platform section (e.g.
``telegram:``) in config.yaml. The bridge loop in
``load_gateway_config`` (gateway/config.py:758) only forwarded a fixed
allowlist of keys, and gateway_restart_notification wasn't on the
list, so:

  1. ``telegram.gateway_restart_notification: false`` never reached
     the platforms data dict.
  2. ``PlatformConfig.from_dict()`` read ``False`` of the safe default
     (``True``) every time.
  3. The suppression check at gateway/run.py:2767 always evaluated to
     truthy → shutdown/restart notifications were always sent.

The issue's suggested fix (route the key through ``bridged``) is not
quite right: ``extra.update(bridged)`` would land it in ``extra``
where ``PlatformConfig.from_dict()`` never looks. ``enabled`` already
solves the same problem by writing directly to ``plat_data`` at the
top level — mirror that path for gateway_restart_notification, and
include it in the "should we materialise this platform at all" guard
so the toggle works even when no other bridgeable keys are present.

Tests cover false / explicit-true / unset (default-true) under
top-level ``telegram:`` and ``discord:`` sections so future
refactoring of the bridge loop trips a clear regression instead of
silently re-breaking the toggle.

Fixes NousResearch#24644
@teknium1

Copy link
Copy Markdown
Contributor

This is implemented on current main. Automated hermes-sweeper review found that the linked bug #24644 has already been fixed by a later main-branch commit.

  • gateway/config.py:980 bridges gateway_restart_notification from per-platform YAML sections into the platform data.
  • gateway/config.py:361-367 makes PlatformConfig.from_dict() read the bridged value from extra as well as the top-level field.
  • gateway/run.py:3980 and the related lifecycle notification paths suppress sends when platform_cfg.gateway_restart_notification is false.
  • The implementation was added by 7a46c68857367e5eb4bc1dfe8838eea6d5c93c50 (fix(gateway): bridge gateway_restart_notification from YAML platform sections) and is contained in v2026.5.28 and later.

@teknium1 teknium1 closed this Jun 11, 2026
@teknium1 teknium1 added the sweeper:implemented-on-main Sweeper: behavior already present on current main label Jun 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/config Config system, migrations, profiles comp/gateway Gateway runner, session dispatch, delivery P3 Low — cosmetic, nice to have sweeper:implemented-on-main Sweeper: behavior already present on current main type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: gateway_restart_notification config setting silently ignored

3 participants