fix(gateway): bridge gateway_restart_notification from config.yaml platform sections (#24644)#24661
Closed
briandevans wants to merge 1 commit into
Closed
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a configuration-loading bug in the Gateway: the per-platform gateway_restart_notification flag (set under top-level platform sections like telegram: in config.yaml) was not being bridged into PlatformConfig, so restart/shutdown notification suppression in gateway/run.py never activated.
Changes:
- Bridge
gateway_restart_notificationfrom top-levelconfig.yamlplatform sections into the per-platform config dict as a first-classPlatformConfigfield (not underextra). - Prevent the top-level platform section from being dropped by the early-
continuewhengateway_restart_notificationis the only key present. - Add regression tests covering: bridged false, default true when absent, and the “only-key-in-section” edge case.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
gateway/config.py |
Updates load_gateway_config() bridging logic so gateway_restart_notification reaches PlatformConfig as a top-level field and isn’t skipped by the early-continue. |
tests/gateway/test_config.py |
Adds regression tests ensuring the flag is bridged correctly, defaults correctly when absent, and works when it’s the only setting in a platform section. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This was referenced May 13, 2026
452999e to
1b549ce
Compare
Contributor
Author
|
CI audit — all 7 test failures are pre-existing baselines on clean
All seven reproduce on clean |
This was referenced May 16, 2026
…atform sections
The `PlatformConfig.gateway_restart_notification` field was correctly
defined, serialized, and parsed by `from_dict()`, but a user setting
`gateway_restart_notification: false` under a top-level platform section
in `config.yaml` (e.g. `telegram:`) never made it into the platform
dict — `load_gateway_config()`'s bridge list copied keys like
`require_mention`, `dm_policy`, `group_policy` etc. into the platforms
data, but had no entry for `gateway_restart_notification`. The flag
therefore always defaulted to True and the suppression checks at
`gateway/run.py:2767`, `:2810`, `:12801`, `:12860` were unreachable
from config.yaml.
Unlike the other bridged keys, `gateway_restart_notification` is a
top-level dataclass field on `PlatformConfig` (read directly as
`platform_cfg.gateway_restart_notification` in gateway/run.py, not via
`cfg.extra.get(...)`). It can't simply be added to the `bridged` dict
that ends up inside `extra` — it has to land at the same level as
`enabled`. Mirror the `enabled` handling:
- Track `restart_notif_explicit = "gateway_restart_notification" in platform_cfg`
- Include it in the early-`continue` predicate so the platform entry
isn't dropped when this is the only key in the section
- Assign `plat_data["gateway_restart_notification"]` after the
`plat_data` dict is materialized, before bridged keys are folded
into `extra`
The raw value is forwarded as-is and coerced by `PlatformConfig.from_dict`
via `_coerce_bool`, so YAML `false`, `"false"`, `0`, etc. all work.
Three regression tests added under `TestLoadGatewayConfig`:
- `test_bridges_gateway_restart_notification_from_config_yaml` — the
headline case from NousResearch#24644; also asserts the value is NOT in
`extra`, since landing there would silently revert the fix.
- `test_gateway_restart_notification_defaults_true_when_absent` —
preserves the existing default when only unrelated keys are set.
- `test_bridges_gateway_restart_notification_when_only_setting` —
covers the predicate change: a platform section that contains
only `gateway_restart_notification: false` (no `enabled`, no
other bridgeable keys) still creates the platform entry instead
of being skipped by the `not bridged and not enabled_was_explicit`
early-`continue`.
Regression guard: with the `gateway/config.py` change stashed out, the
two bridge tests fail with `tg_cfg is None` / `slack_cfg is None`;
re-applied, they pass. The defaults-true test passes in both states
(it asserts the existing default path, not the new bridge).
Fixes NousResearch#24644
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1b549ce to
5eb8a3a
Compare
Contributor
Author
|
Closing — superseded by colin-chang's commit |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Wires
gateway_restart_notification: false(under a top-level platform section liketelegram:inconfig.yaml) into the platform'sPlatformConfigso the lifecycle-notification suppression atgateway/run.py:2767/:2810/:12801/:12860actually fires. The feature was introduced in #20892 but the config bridge inload_gateway_config()was never updated, so setting the flag inconfig.yamlhad no effect — confirmed by the issue's gateway log (notifications sent despite the setting).PlatformConfig.gateway_restart_notification(gateway/config.py:281) is a dataclass field, serialized into_dict(), parsed byfrom_dict()via_coerce_bool. Consumers ingateway/run.pyread it directly asplatform_cfg.gateway_restart_notification. Butload_gateway_config()(gateway/config.py:758–825) — the bridge that copies top-leveltelegram:/slack:/etc. keys fromconfig.yamlinto the internal platforms dict — has an allow-listed bridge set with no entry forgateway_restart_notification.The issue's suggested snippet would land the value inside
extra(the bridge folds intoextra.update(bridged)), butgateway_restart_notificationis a top-levelPlatformConfigfield, not anextrakey. Mirrors theenabledhandling instead: trackrestart_notif_explicit, include it in the early-continuepredicate so a section containing only this key isn't skipped, and assignplat_data["gateway_restart_notification"]directly alongsideplat_data["enabled"]._coerce_boolalready coversfalse/"false"/0.Related Issue
Fixes #24644
Type of Change
Changes Made
gateway/config.py— inload_gateway_config(), trackrestart_notif_explicit = "gateway_restart_notification" in platform_cfg, include it in the early-continuepredicate, and assignplat_data["gateway_restart_notification"]directly (not viabridged/extra).tests/gateway/test_config.py— 3 new cases inTestLoadGatewayConfig: bridges from config.yaml, defaults true when absent, bridges when only setting in the section. Belt-and-braces assertion that the value does NOT land intg_cfg.extra.How to Test
uv run --with pytest --with pytest-xdist --with pytest-asyncio python3 -m pytest tests/gateway/test_config.py tests/gateway/test_restart_notification.py tests/gateway/test_restart_drain.py -vtests/gateway/test_telegram_group_gating.py tests/gateway/test_slack_mention.py— 72 passed.gateway/config.pystashed out, the two bridge tests fail withtg_cfg is None/slack_cfg is None; re-applied, they pass.Checklist
Code
fix(scope):,feat(scope):, etc.)Documentation & Housekeeping
docs/, docstrings) — N/Acli-config.yaml.exampleif I added/changed config keys — N/A, key already documented from feat(gateway): per-platform gateway_restart_notification flag #20892CONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — N/ARelated / Positioning