fix: address self-review findings on the check_fn/ensure_deps_fn split
- gateway/config.py: rewrite the stale enablement-pass header comment that still described check_fn as 'the single source of truth for are-my-env- vars-set' / 'lazy-installs it' — both false under the new contract. - teams: check_requirements docstring wrongly claimed credential checks (body checks only SDK/aiohttp presence); derive install_hint from the canonical LAZY_DEPS pins + sys.executable instead of hardcoding '~/.hermes/hermes-agent/venv/bin/pip' and version pins (wrong under HERMES_HOME overrides / profile installs; pins go stale on CVE bumps); connect() fatal-error hints now point at the venv pip instead of bare system pip (the PEP 668 trap the docs warn about). - teams docs: drop exact version pins from the two manual-install commands (LAZY_DEPS is the source of truth; unpinned installs still work and the text can't go stale). - hermes_cli/status.py: per-entry exception guard around check_fn so one raising probe can't abort the listing of all remaining plugin platforms (aligns with the other three call sites). - tests: rename test_register_check_fn_is_active_lazy_installer -> test_register_splits_passive_probe_from_active_installer (name said the opposite of what it verifies).
This commit is contained in:
@@ -2512,10 +2512,11 @@ def _apply_env_overrides(config: GatewayConfig) -> None:
|
||||
pass
|
||||
|
||||
# Registry-driven enable for plugin platforms. Built-ins have explicit
|
||||
# blocks above; plugins expose check_fn() which is the single source of
|
||||
# truth for "are my env vars set?". When it returns True, ensure the
|
||||
# platform is enabled so start() will create its adapter. Plugins that
|
||||
# need to seed ``PlatformConfig.extra`` from env vars (e.g. Google Chat's
|
||||
# blocks above. A plugin platform is enabled when its credentials are
|
||||
# configured (``is_connected``) and its dependencies are either present
|
||||
# (passive ``check_fn``) or installable on demand (``ensure_deps_fn``,
|
||||
# run later by ``create_adapter()`` — never here). Plugins that need to
|
||||
# seed ``PlatformConfig.extra`` from env vars (e.g. Google Chat's
|
||||
# project_id / subscription_name) can supply ``env_enablement_fn`` on
|
||||
# their PlatformEntry — called here BEFORE adapter construction.
|
||||
#
|
||||
|
||||
@@ -505,7 +505,13 @@ def show_status(args):
|
||||
try:
|
||||
from gateway.platform_registry import platform_registry
|
||||
for entry in platform_registry.plugin_entries():
|
||||
configured = entry.check_fn()
|
||||
# Per-entry guard: one raising probe must not abort the listing
|
||||
# of every remaining plugin platform (matches the other three
|
||||
# check_fn call sites).
|
||||
try:
|
||||
configured = bool(entry.check_fn())
|
||||
except Exception:
|
||||
configured = False
|
||||
status_str = "configured" if configured else "not configured"
|
||||
label = entry.label
|
||||
print(f" {label:<12} {check_mark(configured)} {status_str} (plugin)")
|
||||
|
||||
@@ -27,6 +27,7 @@ import html
|
||||
import json
|
||||
import logging
|
||||
import os
|
||||
import sys
|
||||
from contextlib import contextmanager
|
||||
from typing import Any, Dict, Iterator, Optional
|
||||
from urllib.parse import quote
|
||||
@@ -417,7 +418,12 @@ class _AiohttpBridgeAdapter:
|
||||
|
||||
|
||||
def check_requirements() -> bool:
|
||||
"""Return True when all Teams dependencies and credentials are present."""
|
||||
"""PASSIVE probe: are the Teams SDK and aiohttp importable right now?
|
||||
|
||||
Never installs anything — credentials are gated separately via
|
||||
``is_connected``/``validate_config``. The ACTIVE lazy-installer is
|
||||
``check_teams_requirements`` (registered as ``ensure_deps_fn``).
|
||||
"""
|
||||
return TEAMS_SDK_AVAILABLE and AIOHTTP_AVAILABLE
|
||||
|
||||
|
||||
@@ -769,13 +775,15 @@ class TeamsAdapter(BasePlatformAdapter):
|
||||
self._conv_refs: Dict[str, Any] = {}
|
||||
|
||||
async def connect(self, *, is_reconnect: bool = False) -> bool:
|
||||
# Lazy-install the Teams SDK on demand (parity with Slack/Discord/etc.),
|
||||
# then re-check the module globals it rebinds.
|
||||
# Defensive re-check: create_adapter() already ran the installer
|
||||
# (ensure_deps_fn) if deps were missing, but connect() can also be
|
||||
# reached via reconnect paths — re-run to bind SDK globals.
|
||||
check_teams_requirements()
|
||||
if not TEAMS_SDK_AVAILABLE:
|
||||
self._set_fatal_error(
|
||||
"MISSING_SDK",
|
||||
"microsoft-teams-apps could not be installed. Run: pip install microsoft-teams-apps",
|
||||
"microsoft-teams-apps could not be installed. "
|
||||
f"Run: {sys.executable} -m pip install microsoft-teams-apps",
|
||||
retryable=False,
|
||||
)
|
||||
return False
|
||||
@@ -783,7 +791,7 @@ class TeamsAdapter(BasePlatformAdapter):
|
||||
if not AIOHTTP_AVAILABLE:
|
||||
self._set_fatal_error(
|
||||
"MISSING_SDK",
|
||||
"aiohttp not installed. Run: pip install aiohttp",
|
||||
f"aiohttp not installed. Run: {sys.executable} -m pip install aiohttp",
|
||||
retryable=False,
|
||||
)
|
||||
return False
|
||||
@@ -1462,6 +1470,28 @@ def interactive_setup() -> None:
|
||||
|
||||
# ── Plugin entry point ────────────────────────────────────────────────────────
|
||||
|
||||
def _install_hint() -> str:
|
||||
"""Build the Teams install hint from the canonical LAZY_DEPS pins.
|
||||
|
||||
Derived (not hardcoded) so a pin bump in ``tools/lazy_deps.py`` — aiohttp
|
||||
is CVE-pinned, so bumps happen — never leaves this string stale, and
|
||||
``sys.executable -m pip`` targets the actual Hermes venv in every layout
|
||||
(default install, ``HERMES_HOME`` override, profile installs) instead of
|
||||
a hardcoded ``~/.hermes`` path. Also sidesteps Ubuntu 24.04's PEP 668
|
||||
``externally-managed-environment`` failure that a bare ``pip install``
|
||||
hint invites.
|
||||
"""
|
||||
try:
|
||||
from tools.lazy_deps import feature_specs
|
||||
specs = " ".join(f"'{s}'" for s in feature_specs("platform.teams"))
|
||||
except Exception: # pragma: no cover — defensive
|
||||
specs = "'microsoft-teams-apps' 'aiohttp'"
|
||||
return (
|
||||
"Teams SDK missing — restart the gateway to auto-install, or run: "
|
||||
f"{sys.executable} -m pip install {specs}"
|
||||
)
|
||||
|
||||
|
||||
def register(ctx) -> None:
|
||||
"""Plugin entry point — called by the Hermes plugin system."""
|
||||
ctx.register_platform(
|
||||
@@ -1477,10 +1507,7 @@ def register(ctx) -> None:
|
||||
validate_config=validate_config,
|
||||
is_connected=is_connected,
|
||||
required_env=["TEAMS_CLIENT_ID", "TEAMS_CLIENT_SECRET", "TEAMS_TENANT_ID"],
|
||||
install_hint=(
|
||||
"Teams SDK missing — restart the gateway to auto-install, or run: "
|
||||
"~/.hermes/hermes-agent/venv/bin/pip install 'microsoft-teams-apps==2.0.13.4' 'aiohttp==3.14.1'"
|
||||
),
|
||||
install_hint=_install_hint(),
|
||||
setup_fn=interactive_setup,
|
||||
# Env-driven auto-configuration — seeds PlatformConfig.extra with
|
||||
# client_id/secret/tenant + port + home_channel so env-only setups
|
||||
|
||||
@@ -299,7 +299,7 @@ class TestTeamsPluginRegistration:
|
||||
kwargs = ctx.register_platform.call_args[1]
|
||||
assert kwargs["name"] == "teams"
|
||||
|
||||
def test_register_check_fn_is_active_lazy_installer(self):
|
||||
def test_register_splits_passive_probe_from_active_installer(self):
|
||||
# check_fn is the PASSIVE probe (status displays call it freely);
|
||||
# the ACTIVE lazy-installer rides on ensure_deps_fn, which
|
||||
# create_adapter() invokes when the passive probe fails (#79812).
|
||||
|
||||
@@ -121,7 +121,7 @@ hermes gateway restart
|
||||
The Teams SDK is optional; when Teams is enabled, the gateway lazy-installs it into Hermes' own venv on first start (do **not** use system `pip install` on Ubuntu 24.04 — that hits PEP 668 `externally-managed-environment`). To install manually into the Hermes venv:
|
||||
|
||||
```bash
|
||||
~/.hermes/hermes-agent/venv/bin/pip install 'microsoft-teams-apps==2.0.13.4' 'aiohttp==3.14.1'
|
||||
~/.hermes/hermes-agent/venv/bin/pip install microsoft-teams-apps aiohttp
|
||||
# or from a clone of the agent: uv sync --extra teams
|
||||
```
|
||||
|
||||
@@ -255,7 +255,7 @@ Make sure your configured port (`TEAMS_PORT`, default `3978`) is reachable from
|
||||
| Problem | Solution |
|
||||
|---------|----------|
|
||||
| `Can't find a suitable configuration file` from `docker compose` | You are not in the repo that has `docker-compose.yml`, or you are on a native install — use `hermes gateway restart` instead, or `cd` into the clone first |
|
||||
| `requirements not met (pip install microsoft-teams-apps …)` / `No adapter available for teams` | Restart gateway so lazy-install can run, or install into the **Hermes venv**: `~/.hermes/hermes-agent/venv/bin/pip install 'microsoft-teams-apps==2.0.13.4' 'aiohttp==3.14.1'`. System `pip` fails on Ubuntu 24.04 (PEP 668) and would not affect the service anyway |
|
||||
| `requirements not met (pip install microsoft-teams-apps …)` / `No adapter available for teams` | Restart gateway so lazy-install can run, or install into the **Hermes venv**: `~/.hermes/hermes-agent/venv/bin/pip install microsoft-teams-apps aiohttp`. System `pip` fails on Ubuntu 24.04 (PEP 668) and would not affect the service anyway |
|
||||
| `health` endpoint works but bot doesn't respond | Check that your tunnel is still running and the bot's messaging endpoint matches the tunnel URL |
|
||||
| `KeyError: 'teams'` in logs | Restart the container — this is fixed in the current version |
|
||||
| Bot responds with auth errors | Verify `TEAMS_CLIENT_ID`, `TEAMS_CLIENT_SECRET`, and `TEAMS_TENANT_ID` are all set correctly |
|
||||
|
||||
Reference in New Issue
Block a user