Skip to content

fix(plugins): warn on missing required env vars and zero-registration (salvage #2768) - #58692

Open
Bartok9 wants to merge 2 commits into
NousResearch:mainfrom
Bartok9:salvage-2768-plugin-env-warn
Open

fix(plugins): warn on missing required env vars and zero-registration (salvage #2768)#58692
Bartok9 wants to merge 2 commits into
NousResearch:mainfrom
Bartok9:salvage-2768-plugin-env-warn

Conversation

@Bartok9

@Bartok9 Bartok9 commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Summary

  • When a plugin declares requires_env but those vars are absent — or when its register() completes without adding any tools/hooks/middleware/commands/CLI commands/other surfaces — the loader now emits a WARNING.
  • Turns a silent no-op into an actionable diagnostic for general plugins.

Motivation

Closes #2765.

Some plugins return early from register() when a required environment variable is unset (or otherwise complete without registering anything), emitting no log of their own. The user sees a plugin that "loaded" but registered zero surfaces, with no clue why. Two warnings cover both angles: declared-but-missing env vars, and completed-but-empty registration.

This is a generic plugin-loader diagnostic, not a Hindsight/memory-provider-specific path. Memory providers are exclusive and skipped by _load_plugin(); Hindsight currently declares requires_env: []. A Hindsight-specific diagnostic would belong in memory-provider discovery if still desired.

Salvage of #2768 by @ygd58

Re-applied to current main. Changes from the original:

  • Env-name extraction handles both requires_env shapes — bare strings and dicts with a name key — because current main allows the rich dict form ({name, description, url, secret}). The original only handled strings.
  • The zero-registration check also considers middleware, slash commands, CLI commands, and secret sources (plus other _extra_registrations surfaces: providers, context engines, platforms, skills, etc.) and exempts deferred platform loaders, which legitimately register nothing at load time.
  • Rebased cleanly onto current main; tests updated to the current tests/hermes_cli/test_plugins.py harness (_make_plugin_dir, PluginManager.discover_and_load).

Review follow-ups (#58692 hermes-sweeper / @teknium1)

Addressed:

  1. CLI-only false positiveregister_cli_command() now counts toward registration; predicate also inspects _cli_commands. Regression: test_cli_only_plugin_no_zero_warning.
  2. Secret-source-only false positive — successful register_secret_source() bumps _extra_registrations. Regression: test_secret_source_only_plugin_no_zero_warning.
  3. Motivation narrowed — dropped Hindsight/systemd-dotenv claims that don't match current load paths (memory providers skip _load_plugin(); gateway already loads Hermes dotenv; fix(gateway): load $HERMES_HOME/.env from systemd unit so plugins see env vars #18606 closed).

Verification

python3 -m pytest tests/hermes_cli/test_plugins.py::TestPluginMissingEnvWarnings -q
8 passed

python3 -m pytest tests/hermes_cli/test_plugins.py -q
124 passed
  • test_missing_requires_env_logs_warning — string-form requires_env
  • test_requires_env_dict_form_logs_warning — dict-form requires_env
  • test_zero_registration_logs_warning — empty register()
  • test_present_requires_env_no_warning / test_registering_plugin_no_zero_warning — negative controls
  • test_extra_registration_no_zero_warning — skill/provider-style surface
  • test_cli_only_plugin_no_zero_warning — CLI command surface
  • test_secret_source_only_plugin_no_zero_warning — secret source surface

@alt-glitch alt-glitch added type/feature New feature or request P3 Low — cosmetic, nice to have comp/cli CLI entry point, hermes_cli/, setup wizard comp/plugins Plugin system and bundled plugins labels Jul 5, 2026

@teknium1 teknium1 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.

Thanks for preserving the generic plugin-loader diagnostic work and updating the manifest parsing/tests.

Problems

  • The stated Hindsight path is not reached: hermes_cli/plugins.py:1605-1614 classifies memory providers as exclusive, and :1397-1410 skips them instead of calling _load_plugin(), where this PR adds both warnings. Hindsight also declares requires_env: [] in plugins/memory/hindsight/plugin.yaml:6.
  • The new predicate at hermes_cli/plugins.py:1822-1829 ignores successful CLI-command registration. register_cli_command() stores entries in _cli_commands (:521-528), while the following DEBUG summary itself counts those commands (:1843-1846). A CLI-only plugin therefore gets a false zero-registration WARNING.
  • Successful register_secret_source() calls are also not represented in _extra_registrations (hermes_cli/plugins.py:802-845), creating the same false warning for secret-source-only plugins.

Suggested changes

  • Include CLI commands and secret sources in registration accounting, with regression tests for each.
  • Move any Hindsight-specific diagnostic to memory-provider discovery, or narrow the PR’s motivation to general plugins. Current gateway startup already loads the Hermes dotenv (gateway/run.py:1334-1339), and the linked systemd root cause was addressed by closed #18606.

Automated hermes-sweeper review.

Comment thread hermes_cli/plugins.py
and not loaded.hooks_registered
and not loaded.middleware_registered
and not loaded.commands_registered
and not ctx._extra_registrations

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.

register_cli_command() writes a valid registration to _cli_commands, but this predicate does not inspect that collection; a CLI-only plugin will receive this zero-registration WARNING. Count CLI commands here and add a CLI-only regression test.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 15, 2026
@Bartok9
Bartok9 force-pushed the salvage-2768-plugin-env-warn branch from d50f5ad to 86e62c9 Compare July 15, 2026 18:49
@Bartok9

Bartok9 commented Jul 15, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the review — addressed in the force-push:

  1. CLI commandsregister_cli_command() now bumps _extra_registrations, and the zero-registration predicate also inspects _cli_commands (matching the existing DEBUG summary). Added test_cli_only_plugin_no_zero_warning.
  2. Secret sources — successful register_secret_source() now increments _extra_registrations. Added test_secret_source_only_plugin_no_zero_warning.
  3. Motivation — PR body narrowed to general plugin-loader diagnostics. Agreed the Hindsight path isn't reached via _load_plugin() (exclusive memory providers + requires_env: []); dropped the systemd/.env claim given gateway dotenv load + closed fix(gateway): load $HERMES_HOME/.env from systemd unit so plugins see env vars #18606.
python3 -m pytest tests/hermes_cli/test_plugins.py::TestPluginMissingEnvWarnings -q
8 passed
python3 -m pytest tests/hermes_cli/test_plugins.py -q
124 passed

… (salvage NousResearch#2768)

Rebuilt on latest main (Bartok9 hygiene 2026-08-01).
Original: NousResearch#58692
@Bartok9

Bartok9 commented Aug 1, 2026

Copy link
Copy Markdown
Contributor Author

Rebuilt onto latest main via patch re-apply (force-push). Please re-run CI.

— Bartok9 public PR hygiene 2026-08-01

@Bartok9
Bartok9 force-pushed the salvage-2768-plugin-env-warn branch from 86e62c9 to ecce4b3 Compare August 1, 2026 17:37
…tok9

Per-PR attribution so check-attribution passes on this branch (Teknium).
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/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Hindsight plugin silently skips tool registration when HINDSIGHT_API_URL is missing/empty

3 participants