From 3633c8690b86d68edfe235fa56abb68dcb52ec29 Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Wed, 13 May 2026 11:59:09 +0530 Subject: [PATCH] refactor(plugins): add apply_yaml_config_fn registry hook MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Lets platform plugins own their YAML→env config bridge instead of forcing core gateway/config.py to know every platform's schema. The hook receives the full parsed config.yaml and the platform's own sub-dict, may mutate os.environ (env > YAML precedence preserved via the standard `not os.getenv(...)` guards), and may return a dict to merge into PlatformConfig.extra. It runs during load_gateway_config() after the existing generic shared-key loop and before _apply_env_overrides(), mirroring the env_enablement_fn dispatch pattern (#21306, #21331). Pure addition — no behavior change for existing platforms. Each of the eight platforms with hardcoded YAML→env blocks today (discord, telegram, whatsapp, slack, dingtalk, mattermost, matrix, feishu, ~252 LOC in gateway/config.py) can migrate in independent follow-up PRs; the hardcoded blocks remain functional in the meantime, and their `not os.getenv(...)` guards make them no-ops for any env var the hook already set. Test coverage: 10 new tests in tests/gateway/test_platform_registry.py covering field default, callable acceptance, env mutation, extras merge, both signature args, exception swallowing, missing/non-dict sections, and env > YAML precedence. Refs #3823, #24356. Closes #24836. --- gateway/config.py | 74 ++++- gateway/platform_registry.py | 16 + gateway/platforms/ADDING_A_PLATFORM.md | 8 + tests/gateway/test_platform_registry.py | 314 ++++++++++++++++++ .../adding-platform-adapters.md | 41 +++ 5 files changed, 444 insertions(+), 9 deletions(-) diff --git a/gateway/config.py b/gateway/config.py index 11bc8b75a0b..a7c742839c0 100644 --- a/gateway/config.py +++ b/gateway/config.py @@ -74,6 +74,24 @@ def _normalize_notice_delivery(value: Any, default: str = "public") -> str: return default +def _ensure_platform_extra_dict(platforms_data: dict, name: str) -> tuple[dict, dict]: + """Get-or-create ``platforms_data[name]`` and its nested ``extra`` dict. + + Both slots are coerced to ``{}`` if a non-dict value is encountered, so + callers can safely write keys without type-checking. Returns + ``(plat_data, extra)`` for in-place mutation. + """ + plat_data = platforms_data.setdefault(name, {}) + if not isinstance(plat_data, dict): + plat_data = {} + platforms_data[name] = plat_data + extra = plat_data.setdefault("extra", {}) + if not isinstance(extra, dict): + extra = {} + plat_data["extra"] = extra + return plat_data, extra + + # Module-level cache for bundled platform plugin names (lives outside the # enum so it doesn't become an accidental enum member). _Platform__bundled_plugin_names: Optional[set] = None @@ -755,7 +773,27 @@ def load_gateway_config() -> GatewayConfig: merged["extra"] = merged_extra platforms_data[plat_name] = merged gw_data["platforms"] = platforms_data - for plat in Platform: + # Iterate built-in platforms plus any registered plugin platforms + # so plugin authors get the same shared-key bridging (#24836). + try: + from hermes_cli.plugins import discover_plugins + discover_plugins() # idempotent + from gateway.platform_registry import platform_registry as _pr + except Exception as e: + logger.debug("plugin discovery skipped: %s", e) + _pr = None + + _shared_loop_targets: list = list(Platform) + if _pr is not None: + for _entry in _pr.plugin_entries(): + try: + _plat = Platform(_entry.name) + except (ValueError, KeyError): + continue + if _plat not in _shared_loop_targets: + _shared_loop_targets.append(_plat) + + for plat in _shared_loop_targets: if plat == Platform.LOCAL: continue platform_cfg = yaml_cfg.get(plat.value) @@ -810,20 +848,38 @@ def load_gateway_config() -> GatewayConfig: enabled_was_explicit = "enabled" in platform_cfg if not bridged and not enabled_was_explicit: continue - plat_data = platforms_data.setdefault(plat.value, {}) - if not isinstance(plat_data, dict): - plat_data = {} - platforms_data[plat.value] = plat_data + plat_data, extra = _ensure_platform_extra_dict(platforms_data, plat.value) if enabled_was_explicit: plat_data["enabled"] = platform_cfg["enabled"] - extra = plat_data.setdefault("extra", {}) - if not isinstance(extra, dict): - extra = {} - plat_data["extra"] = extra if plat == Platform.SLACK and enabled_was_explicit: extra["_enabled_explicit"] = True extra.update(bridged) + # Plugin-owned YAML→env config bridges (#24836). See + # ``PlatformEntry.apply_yaml_config_fn`` for the hook contract. + # Order: shared-key loop (above) → this dispatch → legacy hardcoded + # blocks (below; no-op when a hook already set their env var) → + # ``_apply_env_overrides()`` after ``GatewayConfig.from_dict``. + if _pr is not None: + for entry in _pr.all_entries(): + if entry.apply_yaml_config_fn is None: + continue + platform_cfg = yaml_cfg.get(entry.name) + if not isinstance(platform_cfg, dict): + continue + try: + seeded = entry.apply_yaml_config_fn(yaml_cfg, platform_cfg) + except Exception as e: + logger.debug( + "apply_yaml_config_fn for %s raised: %s", + entry.name, e, + ) + continue + if not isinstance(seeded, dict) or not seeded: + continue + _, extra = _ensure_platform_extra_dict(platforms_data, entry.name) + extra.update(seeded) + # Slack settings → env vars (env vars take precedence) slack_cfg = yaml_cfg.get("slack", {}) if isinstance(slack_cfg, dict): diff --git a/gateway/platform_registry.py b/gateway/platform_registry.py index 96bfe1ccadf..97f0c0e1d74 100644 --- a/gateway/platform_registry.py +++ b/gateway/platform_registry.py @@ -119,6 +119,22 @@ class PlatformEntry: # Signature: () -> Optional[dict[str, Any]] env_enablement_fn: Optional[Callable[[], Optional[dict]]] = None + # ── YAML→env config bridge ── + # Optional: translate this platform's ``config.yaml`` keys into env vars + # and/or seed ``PlatformConfig.extra`` directly. Lets a plugin own its + # YAML config translation instead of forcing core ``gateway/config.py`` + # to know every platform's schema. + # + # Signature: (yaml_cfg: dict, platform_cfg: dict) -> Optional[dict] + # Called from ``load_gateway_config()`` after the generic shared-key loop + # and before ``_apply_env_overrides``. Mutating ``os.environ`` is allowed + # (use ``not os.getenv(...)`` guards to preserve env > YAML precedence); + # any returned dict is merged into ``PlatformConfig.extra``. Exceptions + # are caught and logged at debug level. + # See website/docs/developer-guide/adding-platform-adapters.md for the + # full contract and a worked example. + apply_yaml_config_fn: Optional[Callable[[dict, dict], Optional[dict]]] = None + # Optional: home-channel env var name for cron/notification delivery # (e.g. ``"IRC_HOME_CHANNEL"``). When set, ``cron.scheduler`` treats this # platform as a valid ``deliver=`` target and reads the env var to diff --git a/gateway/platforms/ADDING_A_PLATFORM.md b/gateway/platforms/ADDING_A_PLATFORM.md index ffe67e046b1..c373b9fa0b9 100644 --- a/gateway/platforms/ADDING_A_PLATFORM.md +++ b/gateway/platforms/ADDING_A_PLATFORM.md @@ -21,6 +21,14 @@ status display, gateway setup, and more. constructed. Without this, env-only setups don't surface in `hermes gateway status` or `get_connected_platforms()` until the SDK instantiates. +- `apply_yaml_config_fn: (yaml_cfg, platform_cfg) -> Optional[dict]` — + translate this platform's `config.yaml` keys into env vars and/or seed + `PlatformConfig.extra` directly. Lets a plugin own its YAML schema + instead of growing core `gateway/config.py` boilerplate per platform. + Mutating `os.environ` is allowed (use `not os.getenv(...)` guards to + preserve env > YAML precedence); the returned dict is merged into + `PlatformConfig.extra`. Called during `load_gateway_config()` after + the generic shared-key loop and before `_apply_env_overrides()`. - `cron_deliver_env_var: str` — name of the `*_HOME_CHANNEL` env var. When set, `deliver=` cron jobs route to this var without editing `cron/scheduler.py`'s hardcoded sets. diff --git a/tests/gateway/test_platform_registry.py b/tests/gateway/test_platform_registry.py index e6bb823aa6c..4ddc645b7b2 100644 --- a/tests/gateway/test_platform_registry.py +++ b/tests/gateway/test_platform_registry.py @@ -394,3 +394,317 @@ class TestPlatformsMerge: assert "LabelTest" in label finally: _reg.unregister("labeltest") + + +# ── apply_yaml_config_fn (PlatformEntry field + load_gateway_config dispatch) ── + + +class TestApplyYamlConfigFnField: + """The hook field itself — defaults, custom values, signature.""" + + def test_default_is_none(self): + entry = PlatformEntry( + name="test", + label="Test", + adapter_factory=lambda cfg: None, + check_fn=lambda: True, + ) + assert entry.apply_yaml_config_fn is None + + def test_accepts_callable(self): + def _hook(yaml_cfg, platform_cfg): + return None + + entry = PlatformEntry( + name="test", + label="Test", + adapter_factory=lambda cfg: None, + check_fn=lambda: True, + apply_yaml_config_fn=_hook, + ) + assert entry.apply_yaml_config_fn is _hook + # Sanity-check the signature contract. + assert entry.apply_yaml_config_fn({"x": 1}, {"y": 2}) is None + + +class TestApplyYamlConfigFnDispatch: + """End-to-end dispatch through load_gateway_config(). + + Each test registers a temporary PlatformEntry, writes a config.yaml in + a tmp HERMES_HOME, calls load_gateway_config(), and asserts the hook + was invoked correctly. Cleanup unregisters the entry. + """ + + def _write_config(self, tmp_path, content: str): + hermes_home = tmp_path / ".hermes" + hermes_home.mkdir() + (hermes_home / "config.yaml").write_text(content, encoding="utf-8") + return hermes_home + + def _register_hook(self, name, hook_fn): + from gateway.platform_registry import platform_registry as _reg + + entry = PlatformEntry( + name=name, + label=name.title(), + adapter_factory=lambda cfg: None, + check_fn=lambda: True, + source="plugin", + apply_yaml_config_fn=hook_fn, + ) + _reg.register(entry) + return _reg + + def test_hook_can_mutate_environ(self, tmp_path, monkeypatch): + """A hook that mutates os.environ has its env vars set after load.""" + env_var = "MYHOOKPLAT_FLAG" + monkeypatch.delenv(env_var, raising=False) + + def _hook(yaml_cfg, platform_cfg): + if "flag" in platform_cfg and not os.getenv(env_var): + os.environ[env_var] = str(platform_cfg["flag"]).lower() + return None + + reg = self._register_hook("myhookplat", _hook) + try: + home = self._write_config( + tmp_path, "myhookplat:\n flag: true\n", + ) + monkeypatch.setenv("HERMES_HOME", str(home)) + + from gateway.config import load_gateway_config + load_gateway_config() + + assert os.environ.get(env_var) == "true" + finally: + reg.unregister("myhookplat") + os.environ.pop(env_var, None) + + def test_hook_returned_dict_merges_into_extra(self, tmp_path, monkeypatch): + """A hook that returns a dict has it merged into PlatformConfig.extra.""" + + def _hook(yaml_cfg, platform_cfg): + return {"seeded_key": "seeded_value", "flag": platform_cfg.get("flag")} + + reg = self._register_hook("myextraplat", _hook) + try: + home = self._write_config( + tmp_path, "myextraplat:\n flag: yes\n", + ) + monkeypatch.setenv("HERMES_HOME", str(home)) + + from gateway.config import load_gateway_config + cfg = load_gateway_config() + + plat = Platform("myextraplat") + assert plat in cfg.platforms + extra = cfg.platforms[plat].extra + assert extra.get("seeded_key") == "seeded_value" + # flag value carried through from yaml_cfg arg. + assert extra.get("flag") is True + finally: + reg.unregister("myextraplat") + + def test_hook_receives_full_yaml_and_platform_subdict( + self, tmp_path, monkeypatch + ): + """Hook receives both the full yaml_cfg and its own platform sub-dict.""" + captured: dict = {} + + def _hook(yaml_cfg, platform_cfg): + captured["yaml_cfg"] = yaml_cfg + captured["platform_cfg"] = platform_cfg + return None + + reg = self._register_hook("mycaptureplat", _hook) + try: + home = self._write_config( + tmp_path, + "top_level_key: 1\n" + "mycaptureplat:\n" + " inner_key: deep\n", + ) + monkeypatch.setenv("HERMES_HOME", str(home)) + + from gateway.config import load_gateway_config + load_gateway_config() + + assert captured["yaml_cfg"].get("top_level_key") == 1 + assert captured["platform_cfg"] == {"inner_key": "deep"} + finally: + reg.unregister("mycaptureplat") + + def test_hook_exception_swallowed(self, tmp_path, monkeypatch): + """A misbehaving hook never aborts load_gateway_config().""" + + def _bad_hook(yaml_cfg, platform_cfg): + raise RuntimeError("plugin author bug") + + # Also register a well-behaved hook to ensure dispatch continues + # iterating after a bad one. + good_called = {"count": 0} + + def _good_hook(yaml_cfg, platform_cfg): + good_called["count"] += 1 + return None + + from gateway.platform_registry import platform_registry as _reg + _reg.register(PlatformEntry( + name="mybadplat", + label="MyBad", + adapter_factory=lambda cfg: None, + check_fn=lambda: True, + source="plugin", + apply_yaml_config_fn=_bad_hook, + )) + _reg.register(PlatformEntry( + name="mygoodplat", + label="MyGood", + adapter_factory=lambda cfg: None, + check_fn=lambda: True, + source="plugin", + apply_yaml_config_fn=_good_hook, + )) + try: + home = self._write_config( + tmp_path, + "mybadplat:\n k: v\n" + "mygoodplat:\n k: v\n", + ) + monkeypatch.setenv("HERMES_HOME", str(home)) + + # Must not raise. + from gateway.config import load_gateway_config + load_gateway_config() + + assert good_called["count"] == 1 + finally: + _reg.unregister("mybadplat") + _reg.unregister("mygoodplat") + + def test_hook_skipped_when_platform_section_missing( + self, tmp_path, monkeypatch + ): + """Hook is NOT called when the platform's YAML section is absent.""" + called = {"count": 0} + + def _hook(yaml_cfg, platform_cfg): + called["count"] += 1 + return None + + reg = self._register_hook("myabsentplat", _hook) + try: + home = self._write_config(tmp_path, "telegram:\n k: v\n") + monkeypatch.setenv("HERMES_HOME", str(home)) + + from gateway.config import load_gateway_config + load_gateway_config() + + assert called["count"] == 0 + finally: + reg.unregister("myabsentplat") + + def test_hook_skipped_when_platform_section_not_dict( + self, tmp_path, monkeypatch + ): + """Hook is NOT called when the platform's YAML section isn't a dict.""" + called = {"count": 0} + + def _hook(yaml_cfg, platform_cfg): + called["count"] += 1 + return None + + reg = self._register_hook("mybadshapeplat", _hook) + try: + home = self._write_config( + tmp_path, "mybadshapeplat: just-a-string\n", + ) + monkeypatch.setenv("HERMES_HOME", str(home)) + + from gateway.config import load_gateway_config + load_gateway_config() + + assert called["count"] == 0 + finally: + reg.unregister("mybadshapeplat") + + def test_env_var_takes_precedence_when_hook_uses_getenv_guard( + self, tmp_path, monkeypatch + ): + """The standard `not os.getenv(...)` guard preserves env > YAML.""" + env_var = "MYPRECPLAT_FLAG" + monkeypatch.setenv(env_var, "preexisting") + + def _hook(yaml_cfg, platform_cfg): + if "flag" in platform_cfg and not os.getenv(env_var): + os.environ[env_var] = str(platform_cfg["flag"]).lower() + return None + + reg = self._register_hook("myprecplat", _hook) + try: + home = self._write_config( + tmp_path, "myprecplat:\n flag: yaml-value\n", + ) + monkeypatch.setenv("HERMES_HOME", str(home)) + + from gateway.config import load_gateway_config + load_gateway_config() + + # Pre-existing env var was NOT clobbered by the hook. + assert os.environ.get(env_var) == "preexisting" + finally: + reg.unregister("myprecplat") + os.environ.pop(env_var, None) + + +class TestPluginPlatformSharedKeyBridge: + """Plugin-registered platforms get the same shared-key bridging as built-ins. + + Without this, plugin authors using ``apply_yaml_config_fn`` would have to + re-implement bridging for every common key (``unauthorized_dm_behavior``, + ``notice_delivery``, ``reply_prefix``, ``require_mention``, ``dm_policy``, + ``allow_from``, etc.) — defeating the hook's whole point of letting + plugins focus on their *platform-specific* keys. + """ + + def _write_config(self, tmp_path, content: str): + hermes_home = tmp_path / ".hermes" + hermes_home.mkdir() + (hermes_home / "config.yaml").write_text(content, encoding="utf-8") + return hermes_home + + def test_shared_keys_bridged_for_plugin_platform(self, tmp_path, monkeypatch): + """A plugin platform's ``require_mention``/``dm_policy``/etc. flow into + ``PlatformConfig.extra`` without the plugin needing its own bridge.""" + from gateway.platform_registry import platform_registry as _reg + + _reg.register(PlatformEntry( + name="mysharedplat", + label="MySharedPlat", + adapter_factory=lambda cfg: None, + check_fn=lambda: True, + source="plugin", + )) + try: + home = self._write_config( + tmp_path, + "mysharedplat:\n" + " require_mention: true\n" + " dm_policy: allow\n" + " reply_prefix: \"→ \"\n" + " allow_from: [\"alice\", \"bob\"]\n", + ) + monkeypatch.setenv("HERMES_HOME", str(home)) + + from gateway.config import load_gateway_config, Platform + cfg = load_gateway_config() + + plat = Platform("mysharedplat") + assert plat in cfg.platforms + extra = cfg.platforms[plat].extra + assert extra.get("require_mention") is True + assert extra.get("dm_policy") == "allow" + assert extra.get("reply_prefix") == "→ " + assert extra.get("allow_from") == ["alice", "bob"] + finally: + _reg.unregister("mysharedplat") diff --git a/website/docs/developer-guide/adding-platform-adapters.md b/website/docs/developer-guide/adding-platform-adapters.md index f3597dfca39..a8433fcacdd 100644 --- a/website/docs/developer-guide/adding-platform-adapters.md +++ b/website/docs/developer-guide/adding-platform-adapters.md @@ -182,6 +182,7 @@ When you call `ctx.register_platform()`, the following integration points are ha | Connected platform validation | Registry `validate_config()` called | | User authorization | `allowed_users_env` / `allow_all_env` checked | | Env-only auto-enable | `env_enablement_fn` seeds `PlatformConfig.extra` + `home_channel` | +| YAML config bridge | `apply_yaml_config_fn` translates `config.yaml` keys into env vars / extras | | Cron delivery | `cron_deliver_env_var` makes `deliver=` work | | `hermes config` UI entries | `requires_env` / `optional_env` in `plugin.yaml` auto-populate | | send_message tool | Routes through live gateway adapter | @@ -239,6 +240,46 @@ def register(ctx): ) ``` + +## YAML→env Config Bridge + +Some users prefer setting `config.yaml` keys (`my_platform.require_mention`, `my_platform.allowed_channels`, etc.) over env vars. The `apply_yaml_config_fn` hook lets your plugin own this translation instead of forcing core `gateway/config.py` to know your platform's YAML schema. + +```python +import os + +def _apply_yaml_config(yaml_cfg: dict, platform_cfg: dict) -> dict | None: + """Translate config.yaml `my_platform:` keys into env vars / extras. + + yaml_cfg — the full top-level parsed config.yaml dict + platform_cfg — the platform's own sub-dict (yaml_cfg.get("my_platform", {})) + + May mutate os.environ directly (use `not os.getenv(...)` guards to + preserve env > YAML precedence) and/or return a dict to merge into + PlatformConfig.extra. Return None or {} for no extras. + """ + if "require_mention" in platform_cfg and not os.getenv("MY_PLATFORM_REQUIRE_MENTION"): + os.environ["MY_PLATFORM_REQUIRE_MENTION"] = str(platform_cfg["require_mention"]).lower() + allowed = platform_cfg.get("allowed_channels") + if allowed is not None and not os.getenv("MY_PLATFORM_ALLOWED_CHANNELS"): + if isinstance(allowed, list): + allowed = ",".join(str(v) for v in allowed) + os.environ["MY_PLATFORM_ALLOWED_CHANNELS"] = str(allowed) + return None # nothing extra to merge into PlatformConfig.extra + +def register(ctx): + ctx.register_platform( + name="my_platform", + ..., + apply_yaml_config_fn=_apply_yaml_config, + ) +``` + +The hook is invoked during `load_gateway_config()` after the generic shared-key loop (which handles common keys like `unauthorized_dm_behavior`, `notice_delivery`, `reply_prefix`, `require_mention`, etc.) and before `_apply_env_overrides()`, so your plugin only needs to bridge **platform-specific** keys. + +Exceptions raised by the hook are swallowed and logged at debug level — a misbehaving plugin never aborts gateway config load. + + ## Cron Delivery To let `deliver=my_platform` cron jobs route to a configured home channel, set `cron_deliver_env_var` to the env var name that holds the default chat/room/channel ID: