mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-06-01 07:01:41 +00:00
fix(gateway): seed plugin extras before is_connected gate (#31703)
Follow-up to 54e61f933. The plugin enablement gate calls
``entry.is_connected(probe_cfg)`` BEFORE ``env_enablement_fn`` runs,
and the probe is built as ``existing_cfg or PlatformConfig()`` — empty
extras, ``enabled=False``.
For plugins whose ``is_connected`` reads ``config.extra`` instead
of env vars directly, that probe is a misrepresentation of what the
platform will look like after enablement. Google Chat's
``_is_connected`` short-circuits on ``config.enabled`` and inspects
``config.extra["project_id"]`` / ``config.extra["subscription_name"]``
— both False on the default probe even when the user has set
``GOOGLE_CHAT_PROJECT_ID`` and ``GOOGLE_CHAT_SUBSCRIPTION_NAME``. Result:
Google Chat silently fails the gate on every env-var-only setup.
Build a candidate probe that mirrors what the platform will look like
post-enablement:
- pre-call ``env_enablement_fn`` and layer its result into the probe's
``extra`` (without mutating any existing platform config)
- pass ``enabled=True`` on the probe — we're asking "would this BE
configured if we let it in?" not "is it currently enabled?"
- reuse the same seeded extras when we commit the platform to
``config.platforms`` (avoids calling ``env_enablement_fn`` twice)
Discord/IRC/Teams/LINE/ntfy/Simplex ``_is_connected`` hooks read env
vars directly, so they are unaffected. This change only restores
Google Chat on env-var-only setups while keeping the original #31116
Discord-no-token block intact.
All 6 shipped ``env_enablement_fn`` implementations were audited and
are pure reads (no ``os.environ`` writes), so running them earlier in
the loop has no observable side effects.
Tests: 2 new in tests/gateway/test_platform_registry.py covering
extras-seeded-before-is_connected and don't-leak-extras-on-gate-fail.
693 tests across 11 adjacent suites pass (platform_registry, config,
google_chat, matrix, discord_connect, ntfy_plugin, simplex_plugin,
line_plugin, irc_adapter, teams, gateway_platform_gating).
Refs #31116.
This commit is contained in:
parent
514f5020c7
commit
396ee69032
2 changed files with 157 additions and 26 deletions
|
|
@ -895,3 +895,92 @@ class TestPluginEnablementGate:
|
|||
)
|
||||
finally:
|
||||
_reg.unregister("myexplicitplat")
|
||||
|
||||
def test_is_connected_sees_env_seeded_extras(self, tmp_path, monkeypatch):
|
||||
"""``env_enablement_fn`` extras must be visible to ``is_connected``.
|
||||
|
||||
Some plugins (e.g. Google Chat) implement ``is_connected`` by
|
||||
inspecting ``config.extra`` (where ``env_enablement_fn`` deposits
|
||||
env-var-derived state) rather than reading ``os.environ`` directly.
|
||||
If the gate runs BEFORE the seeding step, those plugins fail the
|
||||
gate even when the user is genuinely configured via env vars.
|
||||
|
||||
Pin the contract: when both hooks are present, ``env_enablement_fn``
|
||||
feeds a candidate config to ``is_connected``.
|
||||
"""
|
||||
from gateway.platform_registry import platform_registry as _reg
|
||||
|
||||
seen_extras: dict = {}
|
||||
|
||||
def _is_connected(cfg):
|
||||
seen_extras["snapshot"] = dict(getattr(cfg, "extra", {}) or {})
|
||||
extra = getattr(cfg, "extra", {}) or {}
|
||||
return bool(extra.get("project_id") and extra.get("subscription_name"))
|
||||
|
||||
def _env_enablement():
|
||||
return {"project_id": "p", "subscription_name": "s"}
|
||||
|
||||
_reg.register(PlatformEntry(
|
||||
name="myextrasplat",
|
||||
label="MyExtras",
|
||||
adapter_factory=lambda cfg: None,
|
||||
check_fn=lambda: True,
|
||||
is_connected=_is_connected,
|
||||
env_enablement_fn=_env_enablement,
|
||||
source="plugin",
|
||||
))
|
||||
try:
|
||||
home = self._write_config(tmp_path)
|
||||
monkeypatch.setenv("HERMES_HOME", str(home))
|
||||
|
||||
from gateway.config import load_gateway_config, Platform
|
||||
cfg = load_gateway_config()
|
||||
|
||||
plat = Platform("myextrasplat")
|
||||
assert plat in cfg.platforms, (
|
||||
"is_connected was called with empty extras — "
|
||||
"env_enablement_fn must seed the probe BEFORE the gate"
|
||||
)
|
||||
assert cfg.platforms[plat].enabled is True
|
||||
# extras populated on the live config too
|
||||
assert cfg.platforms[plat].extra.get("project_id") == "p"
|
||||
assert cfg.platforms[plat].extra.get("subscription_name") == "s"
|
||||
# and the probe saw them
|
||||
assert seen_extras["snapshot"]["project_id"] == "p"
|
||||
finally:
|
||||
_reg.unregister("myextrasplat")
|
||||
|
||||
def test_is_connected_failed_gate_does_not_leak_extras(
|
||||
self, tmp_path, monkeypatch
|
||||
):
|
||||
"""When the gate rejects, env-seeded extras must NOT leak onto
|
||||
``config.platforms``. A rejected plugin should be invisible, not
|
||||
present-but-partially-populated.
|
||||
"""
|
||||
from gateway.platform_registry import platform_registry as _reg
|
||||
|
||||
_reg.register(PlatformEntry(
|
||||
name="myrejectedplat",
|
||||
label="MyRejected",
|
||||
adapter_factory=lambda cfg: None,
|
||||
check_fn=lambda: True,
|
||||
is_connected=lambda cfg: False,
|
||||
env_enablement_fn=lambda: {"some_key": "should-not-leak"},
|
||||
source="plugin",
|
||||
))
|
||||
try:
|
||||
home = self._write_config(tmp_path)
|
||||
monkeypatch.setenv("HERMES_HOME", str(home))
|
||||
|
||||
from gateway.config import load_gateway_config, Platform
|
||||
cfg = load_gateway_config()
|
||||
|
||||
plat = Platform("myrejectedplat")
|
||||
if plat in cfg.platforms:
|
||||
assert cfg.platforms[plat].enabled is False
|
||||
assert "some_key" not in cfg.platforms[plat].extra, (
|
||||
"Rejected plugin's env-seeded extras leaked onto "
|
||||
"config.platforms"
|
||||
)
|
||||
finally:
|
||||
_reg.unregister("myrejectedplat")
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue