Skip to content

fix(gateway): split check_fn (passive probe) from ensure_deps_fn (active installer) - #80305

Merged
kshitijk4poor merged 6 commits into
NousResearch:mainfrom
kshitijk4poor:fix/platform-registry-ensure-deps
Aug 7, 2026
Merged

fix(gateway): split check_fn (passive probe) from ensure_deps_fn (active installer)#80305
kshitijk4poor merged 6 commits into
NousResearch:mainfrom
kshitijk4poor:fix/platform-registry-ensure-deps

Conversation

@kshitijk4poor

Copy link
Copy Markdown
Collaborator

Summary

Platform plugins no longer have to choose between "status displays pip-install SDKs" and "lazy-install never runs": PlatformEntry.check_fn is now contractually a PASSIVE dependency probe, and a new optional PlatformEntry.ensure_deps_fn carries the ACTIVE lazy-installer, which create_adapter() runs exactly when the probe fails — right before the gateway connects an enabled, configured platform.

Builds on and supersedes #79812 by @xxxigm (both commits cherry-picked with authorship preserved).

The bug class

check_fn served three contradictory roles: adapter-creation gate (platform_registry.create_adapter()), config auto-enablement gate (gateway/config.py::_apply_env_overrides), and status display (hermes setup / hermes status / dashboard readiness probe). Plugins had to pick ONE function for all three:

Wiring Platforms Failure mode
ACTIVE installer as check_fn discord, slack, telegram, matrix, dingtalk, feishu Every status/config surface could pip-install SDKs as a side effect — the desktop 94% boot-loop class (mitigated once in _apply_env_overrides, re-fixed in _platform_status, still latent anywhere new that reads check_fn)
PASSIVE probe as check_fn teams, wecom_callback create_adapter() returns None before connect() can lazy-install → SDK never installs. #79812 for Teams; wecom_callback's platform.wecom_callback LAZY_DEPS entry was unreachable dead code

The contributor's fix (#79812) swapped Teams to the active-as-check_fn wiring — correct for the deadlock, but it moves Teams into the first row: hermes gateway status and load_gateway_config() fallbacks would pip-install microsoft-teams-apps as a side effect. This PR fixes the class instead of the instance.

Changes

  • gateway/platform_registry.py: ensure_deps_fn field; create_adapter() probes, then installs, then re-gates. Exception-safe on both probe and installer.
  • gateway/config.py: enablement pass keeps a configured platform whose deps are missing but installable; the install itself is deferred to create_adapter() at gateway start (never runs during config load — preserves the desktop boot-loop fix by construction).
  • hermes_cli/plugins.py: register_platform docstring documents the PASSIVE/ACTIVE contract (ensure_deps_fn flows via entry_kwargs).
  • Migrated all 8 lazy-installable platform plugins:
    • teams: check_fn=check_requirements (passive), ensure_deps_fn=check_teams_requirements (active) — fixes fix(teams): lazy-install SDK when gateway starts Teams #79812 without install-on-status
    • telegram/discord/slack/matrix/dingtalk/feishu: new *_deps_present() passive probes; existing check_*_requirements installers moved to ensure_deps_fn
    • wecom_callback: gains ensure_wecom_callback_requirements — its lazy-install path works for the first time
  • Platforms with no optional deps (irc, ntfy, buzz, simplex, line, a2a, …) unchanged: no ensure_deps_fn means a False check_fn stays a hard block.
  • Docs: adding-platform-adapters.md documents the two-field contract; Teams messaging docs (from fix(teams): lazy-install SDK when gateway starts Teams #79812) cover native/systemd vs Docker start and Hermes-venv installs.

Validation

Check Result
tests/gateway/ registry/teams/lazy-install selection 120 passed, 2 skipped
New registry tests (TestEnsureDepsFn, enablement gates) 8 new tests, all passing
E2E (real imports, temp HERMES_HOME, all 8 plugins) 28/28: wiring verified per-plugin; installer runs exactly once on missing deps; blocks on failed install; Teams passive probe provably never calls lazy_deps.ensure; config enablement enables-but-never-installs; _platform_status never installs
ruff on all 15 changed .py files clean

Credit

Teams deadlock diagnosis, original fix, and docs by @xxxigm (#79812) — commits cherry-picked with authorship preserved, then reworked into the two-field split on top.

Closes #79812

@alt-glitch alt-glitch added type/bug Something isn't working comp/gateway Gateway runner, session dispatch, delivery comp/cli CLI entry point, hermes_cli/, setup wizard comp/plugins Plugin system and bundled plugins P2 Medium — degraded but workaround exists sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades labels Aug 6, 2026
@kshitijk4poor
kshitijk4poor enabled auto-merge (rebase) August 6, 2026 16:42
@kshitijk4poor
kshitijk4poor disabled auto-merge August 6, 2026 16:44
@kshitijk4poor
kshitijk4poor enabled auto-merge (rebase) August 6, 2026 16:46
xxxigm and others added 6 commits August 6, 2026 23:47
Platform registry create_adapter() gates on check_fn before the adapter
exists, so wiring the passive probe permanently blocked connect() and
the existing check_teams_requirements() lazy-install never ran.
Step 5 only showed docker compose from a clone; native/systemd users
hit missing compose files and PEP 668 system-pip failures.
…ive installer)

PlatformEntry.check_fn served three contradictory roles: adapter-creation
gate, config auto-enablement gate, and status display. Plugins had to pick
one function for all three:

- Active installer as check_fn (discord/slack/telegram/matrix/dingtalk/
  feishu): every status display could pip-install SDKs as a side effect
  (the desktop 94% boot-loop class).
- Passive probe as check_fn (teams, wecom_callback): create_adapter()
  returned None before connect() could lazy-install, so the SDK never
  installed (NousResearch#79812 deadlock; wecom_callback's platform.wecom_callback
  LAZY_DEPS entry was dead code).

The split makes both call sites correct by construction:

- check_fn is now contractually PASSIVE (probe only, never installs).
- New optional PlatformEntry.ensure_deps_fn is the ACTIVE installer;
  create_adapter() runs it exactly when check_fn is False — the platform
  is enabled+configured and the gateway is about to connect it.
- Config enablement keeps a configured platform whose deps are missing
  but installable; the install itself is deferred to create_adapter().
- Status surfaces (_platform_status, hermes status) read only the
  passive probe and can never trigger pip.

Migrated all lazy-installable platform plugins to the split; platforms
with no optional deps (irc/ntfy/buzz/simplex/line/a2a/...) are unchanged
— no ensure_deps_fn means a False check_fn stays a hard block.
wecom_callback gains a working installer for the first time.

Builds on @xxxigm's NousResearch#79812 (both commits cherry-picked with authorship
preserved), reworking the check_fn swap into the two-field split so the
Teams fix doesn't reintroduce install-on-status.
- 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).
- matrix/dingtalk: extract deps-only installers (ensure_matrix_deps,
  ensure_dingtalk_deps) and register THOSE as ensure_deps_fn — the prior
  check_*_requirements combined credential env checks with the install,
  so a platform configured via PlatformConfig.extra (which is_connected
  accepts) would pass enablement, reach create_adapter(), and have the
  'installer' veto on env-var grounds before installing anything —
  re-creating the NousResearch#79812 deadlock for extra-configured setups.  The
  combined deps+credentials functions remain for setup/status callers.
- matrix/feishu passive probes: use the existing lazy_deps.is_available()
  instead of hand-rolling 'not feature_missing(...)' (reuse finding).
- teams: module docstring no longer recommends bare system pip (the
  PEP 668 trap purged everywhere else); docs troubleshooting row updated
  to match the new hint text.
- wecom_callback: drop dead 'global ET, DEFUSEDXML_AVAILABLE'
  (ensure_and_bind mutates the module dict directly; nothing assigns).
- tests: parametrized wiring contract for all 8 lazy-installable
  platforms — ensure_deps_fn present and distinct from check_fn
  (behavior contract, not identity snapshot, so renames don't churn it).
…pip=True)

Fold the remaining simplify-code reuse finding: teams' _install_hint()
duplicated lazy_deps' spec-fetch + quote + join (feature_install_command
already builds pip commands from LAZY_DEPS). Add a venv_pip=True variant
to feature_install_command — sys.executable -m pip targeting, correct in
every install layout and immune to PEP 668 — and shrink the teams helper
to a one-line call.  Also gives matrix and the other platforms a shared
derived hint to adopt later.  New test mutation-checked (fails when
venv_pip returns the uv form).
@kshitijk4poor
kshitijk4poor force-pushed the fix/platform-registry-ensure-deps branch from 766853a to b7eccc1 Compare August 6, 2026 18:17
auto-merge was automatically disabled August 7, 2026 07:55

Pull request was closed

@kshitijk4poor kshitijk4poor reopened this Aug 7, 2026
@kshitijk4poor
kshitijk4poor enabled auto-merge (rebase) August 7, 2026 07:56
@kshitijk4poor
kshitijk4poor merged commit 99237a4 into NousResearch:main Aug 7, 2026
40 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard comp/gateway Gateway runner, session dispatch, delivery comp/plugins Plugin system and bundled plugins P2 Medium — degraded but workaround exists sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants