fix(gateway): bridge shared-key loop to nested platform config blocks - #35866
Closed
AhmetArif0 wants to merge 1 commit into
Closed
fix(gateway): bridge shared-key loop to nested platform config blocks#35866AhmetArif0 wants to merge 1 commit into
AhmetArif0 wants to merge 1 commit into
Conversation
The shared-key bridging loop (allow_from, require_mention, free_response_channels, …) read only the top-level yaml platform block (yaml_cfg.get(plat.value)). When a user configured a platform solely under ``platforms:`` or ``gateway.platforms:`` with no top-level block, the loop skipped that platform entirely and all bridged keys were silently dropped into PlatformConfig.extra — making allow_from, require_mention, etc. ineffective for nested-only configs. The apply_yaml_config_fn dispatch already received this same fallback in 44f3e51 to handle plugin adapters (e.g. Discord allow_from). The shared-key loop now mirrors it: if yaml_cfg.get(plat.value) is absent, fall back to gateway.platforms.<name> then platforms.<name>. The enabled field is deliberately excluded from the nested fallback (guarded by _cfg_toplevel): _merge_platform_map already merged it with the correct precedence, so re-applying it from a single nested source would overwrite the correctly-merged value. Two new regression tests assert that allow_from and require_mention configured under platforms.telegram and gateway.platforms.telegram are bridged into PlatformConfig.extra. All 54 existing config tests pass.
alt-glitch
added
type/bug
Collaborator
|
Follow-up to merged #35329 which fixed nested platform config loading + Discord allow_from bridging. This PR extends the fix to the shared-key bridging loop (allow_from, require_mention, free_response_channels, etc.) which still read only the top-level yaml block. Related: #34565 (closed), #28245 (open). |
tonydwb
approved these changes
May 31, 2026
tonydwb
left a comment
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: Approved ⚡
✅ What's Good
- Correct fix: The shared-key loop (
_merge_platform_map) now falls back to nestedplatforms:/gateway.platforms:paths when a top-level platform block is absent. This mirrors the identical fix already applied toapply_yaml_config_fn(#44f3e51), ensuring consistency. - Good edge case handling: The
_cfg_toplevelflag correctly distinguishes top-level from nested configs, allowing theenabledfield to remain sourced only from top-level blocks (as_merge_platform_mapalready handles it with correct precedence for nested configs). - Clean tests: Two regression tests (
platforms:andgateway.platforms:paths) with clear, descriptive docstrings that document the bug and the expected behavior. - Proper scope: 83 additions, 1 deletion — focused fix with good test coverage.
Summary
A well-scoped fix for a config parsing correctness bug. No issues found.
Reviewed by Hermes Agent
Contributor
|
Merged via PR #38984. Your commit was cherry-picked onto current main with your authorship preserved in git log (4ae3c98). Thanks for catching the shared-key-loop parity gap — nested-only |
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
4 participants
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.
Summary
The shared-key bridging loop in
load_gateway_config()— which copiesallow_from,require_mention,free_response_channels, and ~20 other keys intoPlatformConfig.extra— read only the top-level yaml platform block (yaml_cfg.get(plat.value)). When a user configured a platform solely underplatforms:orgateway.platforms:without a top-level block, the loop silently skipped the platform and all bridged keys were dropped, making access control and mention settings ineffective for nested-only configs.The
apply_yaml_config_fndispatch already received this same nested fallback in44f3e51to handle plugin platforms (Discordallow_from). The shared-key loop now mirrors it exactly.Why
enabledis excluded from the nested fallback:_merge_platform_mapalready mergedenabledwith the correctplatforms.*>gateway.platforms.*precedence before the loop runs. Re-applying it from a single nested source would overwrite the correctly-merged value, which caused the existingtest_top_level_platforms_override_nested_gateway_platformstest to fail in an earlier version of this fix.Test plan
allow_from+require_mentionconfigured underplatforms.telegramandgateway.platforms.telegram(no top-level block) are correctly bridged intoPlatformConfig.extratests/gateway/test_config.pytests pass, including theenabledprecedence testgateway/config.py(+21 lines),tests/gateway/test_config.py(+62 lines)