Skip to content

fix(gateway): preserve top-level platform config keys#10453

Closed
briandevans wants to merge 2 commits into
NousResearch:mainfrom
briandevans:fix/gateway-platform-extra
Closed

fix(gateway): preserve top-level platform config keys#10453
briandevans wants to merge 2 commits into
NousResearch:mainfrom
briandevans:fix/gateway-platform-extra

Conversation

@briandevans

@briandevans briandevans commented Apr 15, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes gateway/config.py so top-level platform settings under config.yaml are preserved instead of being silently dropped during PlatformConfig.from_dict().

The bug is that load_gateway_config() correctly deep-merges platforms.<name> blocks from config.yaml, but PlatformConfig.from_dict() only kept a fixed allowlist of fields and discarded everything else unless it was nested under extra:. Adapters like webhook and api_server read their settings from config.extra, so shorthand config such as platforms.webhook.port: 9100 never reached the adapter.

This stays narrow at the parser boundary:

  • unknown top-level platform keys are folded into extra
  • explicit nested extra mappings remain backward compatible and still take precedence on overlap
  • invalid non-mapping extra values are ignored with a warning instead of being passed through silently

Related Issue

Fixes #10206

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • Updated gateway/config.py so PlatformConfig.from_dict() copies unknown top-level platform keys into extra
  • Preserved backward compatibility for explicit nested extra values when both shorthand and extra forms are present
  • Added a warning when extra is present but not a mapping, so adapters do not silently receive an invalid shape
  • Added regression coverage in tests/gateway/test_config.py for:
    • parser-level promotion of unknown top-level keys into extra
    • invalid non-mapping extra values being ignored with a warning
    • load_gateway_config() loading platforms.webhook.host / platforms.webhook.port from config.yaml

How to Test

  1. Add this to ~/.hermes/config.yaml:
    platforms:
      webhook:
        enabled: true
        host: 0.0.0.0
        port: 9100
  2. Run hermes gateway (or load config through gateway.config.load_gateway_config()).
  3. Confirm the webhook adapter now sees host=0.0.0.0 and port=9100 instead of falling back to defaults.
  4. Optional validation edge case:
    platforms:
      webhook:
        enabled: true
        extra: not-a-mapping
        port: 9100
    Confirm config loading still preserves port=9100 and logs a warning about the invalid extra value.

Validation

Focused and adjacent checks run locally:

  • source /Users/magic/Documents/Code/hermes-agent/venv/bin/activate && python -m pytest tests/gateway/test_config.py -q20 passed
  • source /Users/magic/Documents/Code/hermes-agent/venv/bin/activate && python -m pytest tests/gateway/test_api_server.py -q114 passed
  • git diff --check → passed
  • source /Users/magic/Documents/Code/hermes-agent/venv/bin/activate && python -m py_compile gateway/config.py tests/gateway/test_config.py → passed

I also ran the repo’s documented broad local test command:

  • source /Users/magic/Documents/Code/hermes-agent/venv/bin/activate && python -m pytest tests/ -q --ignore=tests/integration --ignore=tests/e2e --tb=short -n auto

That command is not clean in this checkout: it ended with 98 failed, 11309 passed, 93 skipped, 60 errors, including missing optional dependencies (acp, fastapi, faster_whisper) and unrelated failures in ACP, web server, Discord, Telegram, and other areas outside this patch.

No separate standalone lint command appears in the repo docs, so I did not invent one.

Tested locally on macOS 26.5 with Python 3.11.

Adjacent surfaces checked:

  • gateway/platforms/webhook.py reads host / port from config.extra
  • gateway/platforms/api_server.py reads host / port from config.extra

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS 26.5

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

Copilot AI review requested due to automatic review settings April 15, 2026 17:42

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes gateway platform config parsing so adapter-specific settings defined as top-level keys under config.yaml platform blocks (e.g. platforms.webhook.port) are preserved by folding them into PlatformConfig.extra, ensuring adapters like webhook and api_server actually receive those values.

Changes:

  • Update PlatformConfig.from_dict() to promote unknown top-level platform keys into extra, with explicit nested extra taking precedence on conflicts.
  • Add regression tests covering both the parser behavior and load_gateway_config() end-to-end bridging from config.yaml.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
gateway/config.py Adjusts platform config parsing to retain unknown top-level keys by merging them into extra.
tests/gateway/test_config.py Adds tests to prevent regressions for shorthand platform config keys flowing into config.extra.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread gateway/config.py
@RuckVibeCodes

Copy link
Copy Markdown

[gus-first-pass] Reviewed PR #10453: fix(gateway): preserve top-level platform config keys. Found potential issues regarding documentation that should be addressed.

@RuckVibeCodes RuckVibeCodes left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change appears necessary; although, ensure to mention the backward compatibility in the documentation.

@briandevans

Copy link
Copy Markdown
Contributor Author

@RuckVibeCodes I updated the PR body to call out backward compatibility explicitly: the existing nested extra: form remains supported and still takes precedence over shorthand top-level platform keys. I also pushed a small follow-up (a27e67f1) to warn on invalid non-mapping extra values and added a regression test for that edge case.

@jleechan2015

Copy link
Copy Markdown

Thanks for fixing this. I hit the same silent-drop bug when configuring platforms.api_server.port at the top level instead of under extra.

My approach was a warning-only fix, but auto-promoting unknown keys into extra (as this PR does) is the better UX — it makes intuitive YAML work without requiring users to know the extra nesting rule. The precedence order (explicit extra > promoted top-level) is also correct.

LGTM — tests/gateway/test_config.py coverage is solid.

@briandevans

Copy link
Copy Markdown
Contributor Author

@jleechan2015 Thanks for the signal-boost! Agreed — warning-only is safer in isolation, but the precedence-aware auto-promote matches how most folks (including you, apparently) actually expect YAML config to behave. If you find any other keys that silently drop, happy to extend the catalog in a follow-up.

@briandevans

Copy link
Copy Markdown
Contributor Author

Closing to keep the queue clean — gateway/config.py has had ~28 commits on main in the last 14 days and this branch is now ~4.8k commits behind, so the surgical fix here has likely been overtaken by the broader config-parsing reshape. Happy to reopen on a fresh branch if the silent-drop bug for top-level platform keys still reproduces on current main.

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 P2 Medium — degraded but workaround exists type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: platforms.webhook.port in config.yaml does not work

5 participants