fix(plugins): warn when required env vars are missing, before register()

[ygd58]

Context

Ports #63050 forward onto current main per @teknium1's review.

Problem

_load_plugin() emitted only a DEBUG registration count, so a plugin whose required env vars were absent loaded silently (issue #2768).

Fix

Normalize requires_env entries (str or dict) before checking the process environment; emit a WARNING naming each missing variable.

Per review, fixes two real bugs in the original port:

1. The check ran AFTER register_fn(ctx). A plugin that reads the missing variable directly during registration raises straight into the generic 'Failed to load plugin' handler before the specific diagnostic is ever reported. Moved the check before register_fn(ctx).
2. The provider-only regression test used a no-op register(), which didn't actually exercise a provider registration. Replaced with a real ImageGenProvider subclass registered via ctx.register_image_gen_provider().

Scope note: this fix is in the general PluginManager loader. The Hindsight memory plugin path uses a separate loader (plugins/memory/init.py) that general discovery skips entirely -- this PR does not touch that path.

Verification

Added a raising-register() regression test and the corrected real-provider test. 5/5 new tests pass; 121/121 in the full tests/hermes_cli/test_plugins.py file.

[teknium1]

Thanks for carrying the str/dict normalization and the pre-register() diagnostic forward. The core missing-env gap remains on current main (hermes_cli/plugins.py:1783-1811).

Problems

• The proposed check is still after module import: the PR loads a directory/entry-point module at hermes_cli/plugins.py:1764-1767, then begins validation at hermes_cli/plugins.py:1779. A plugin that accesses a declared variable at module scope raises into the generic loader handler before the specific diagnostic runs.
• The provider test captures only WARNING records at tests/hermes_cli/test_plugins.py:2533-2540, while the newly added empty-registration message is DEBUG. Its loaded.enabled assertion does not verify that the provider was registered.

Suggested changes

• Validate and warn before module loading, and add a module-import failure regression.
• Assert the named test provider is retrievable from agent.image_gen_registry before cleanup.

Automated hermes-sweeper review.

[teknium1]

This check runs after _load_directory_module() / _load_entrypoint_module() have executed. Move it before module loading so a plugin whose module-level import accesses a missing declared variable receives this specific diagnostic rather than only the generic loader failure.

[teknium1]

loaded.enabled only proves register() returned normally. Assert agent.image_gen_registry.get_provider('test-provider-2768') is non-null so this regression verifies the claimed provider-registration path; the WARNING assertion alone is vacuous because the new message is DEBUG.

[ygd58]

Both were real. Fixed in the latest commit:

1. Moved the missing-env check to run before _load_directory_module()/_load_entrypoint_module() entirely, using manifest.requires_env directly (no dependency on the module having been imported). A plugin that accesses a missing declared variable at module scope now gets the specific WARNING too, not just the generic loader failure.
2. The provider-only test now asserts agent.image_gen_registry.get_provider(...) returns non-null, not just loaded.enabled -- proving register_image_gen_provider() actually ran.

Added the requested module-import-failure regression test. 122/122 pass in the full tests/hermes_cli/test_plugins.py file (no regression).

[GottZ]

This was generated by AI during triage.

Summary

Ten PRs address or reference this two-issue complex. The diffs divide into generic plugin-loader diagnostics (#2768, #58692, #63050, #68703), systemd environment loading (#18606), unavailable-memory-provider diagnostics (#70257), and lifecycle-hook implementations for closed #2817 (#2820, #2823, #2930, #3542).

Related pull requests

• fix(plugins): warn when required env vars are missing or zero tools registered #2768 best fix — (+108/-0) — n/a: Adds missing-required-env and zero-tool/hook warnings, but the keep_open maintainer verdict identifies dict-form requires_env failures and false warnings for valid non-tool registrations. It remains the recorded best fix for Hindsight plugin silently skips tool registration when HINDSIGHT_API_URL is missing/empty #2765, so author action is required to rebase and incorporate the corrected normalization and registration accounting from later variants.
• fix(plugins): wire-plugin-lifecycle-hooks #2820 [closed] fixes — (+100/-0) — n/a: Wires all four missing lifecycle hooks, addressing [Bug]: Plugin hooks pre_llm_call, post_llm_call, on_session_start, on_session_end are documented but never invoked #2817 rather than the configuration failure in Hindsight plugin silently skips tool registration when HINDSIGHT_API_URL is missing/empty #2765; the contributor also documented a streaming-fallback double-fire edge case. Closed, but relevant as a precursor whose functionality was later shipped through feat: activate plugin lifecycle hooks (pre/post_llm_call, session start/end) #3542.
• feat: activate plugin lifecycle hooks (pre/post_llm_call, session start/end) #2823 [closed] fixes — (+144/-15) — n/a: Activates the four lifecycle hooks, collects pre_llm_call return values for ephemeral context, and updates tests and documentation. Closed because its commits were salvaged into merged feat: activate plugin lifecycle hooks (pre/post_llm_call, session start/end) #3542 with authorship preserved.
• feat(plugins): fire pre_llm_call, post_llm_call, on_session_start, on_session_end hooks #2930 [closed] partial — (+561/-1) — n/a: Adds lifecycle-hook calls plus a substantial local-LLM plugin example, but its session hooks are placed in the interactive CLI path rather than the complete conversation lifecycle. Closed and explicitly superseded after feat: activate plugin lifecycle hooks (pre/post_llm_call, session start/end) #3542 shipped the hook implementation.
• feat: activate plugin lifecycle hooks (pre/post_llm_call, session start/end) #3542 [merged] best fix — (+150/-19) — n/a: The merged reference implementation for [Bug]: Plugin hooks pre_llm_call, post_llm_call, on_session_start, on_session_end are documented but never invoked #2817 wires all four lifecycle hooks, supports pre-call context injection, passes shallow history copies, and updates documentation. It remains relevant as the recorded best fix and shipped consolidation of feat: activate plugin lifecycle hooks (pre/post_llm_call, session start/end) #2823.
• fix(gateway): load $HERMES_HOME/.env from systemd unit so plugins see env vars #18606 [closed] partial — (+37/-0) — n/a: Adds optional EnvironmentFile directives to newly generated user and system systemd units, directly targeting the environment-inheritance cause described in Hindsight plugin silently skips tool registration when HINDSIGHT_API_URL is missing/empty #2765 but not adding a runtime provider diagnostic. Closed, but relevant as the concrete unit-template root-cause patch.
• fix(plugins): warn on missing required env vars and zero-registration (salvage #2768) #58692 partial — (+267/-0) — n/a: Salvages fix(plugins): warn when required env vars are missing or zero tools registered #2768 with str/dict requires_env normalization and broad accounting for provider, command, secret-source, and other registration surfaces; its diff still does not reach the separate Hindsight memory-provider loader. Despite its keep_open review, the shown follow-up diff addresses the cited CLI-command and secret-source false positives, while the rebuilt head has no fresh CI result in the supplied delta.
• fix(plugins): warn when required env vars are missing, handle str/dict manifest entries #63050 [closed] duplicate — (+165/-4) — n/a: Adds normalized generic missing-env warnings, but places the check after register() and uses a no-op as its claimed provider-only test. Closed in favor of fix(plugins): warn when required env vars are missing, before register() #68703, which explicitly ports this work forward and corrects those reviewed defects.
• fix(plugins): warn when required env vars are missing, before register() #68703 partial — (+297/-4) — n/a: Moves normalized str/dict required-env validation before module import and adds regressions for module-scope failure and registry-backed provider registration; it intentionally does not modify Hindsight's separate memory-provider loader. Despite the keep_open review on fix(plugins): warn when required env vars are missing, before register() #68703, the current diff directly addresses both cited defects, making this the strongest salvage source for the generic-loader portion.
• fix(memory): warn when a configured provider reports unavailable (#2765) #70257 [closed] best fix — (+114/-0) — n/a: Adds a deduplicated warning at the actual configured-provider is_available()==False branch and surfaces the systemd/.env diagnosis in hermes memory status, directly covering the current silent Hindsight drop. Closed as superseded by combined feat(hindsight): memory provider improvements — recall_sync, retain_source, setup templates, memory indicators, error hints #74379, but still relevant as the recorded best-fix reference for this provider path.

Duplicates

#2768, #58692, #63050, and #68703 substantially overlap on generic required-env diagnostics; #63050 is explicitly superseded by #68703, while #58692 is a salvage of #2768. #2820, #2823, #2930, and #3542 overlap on lifecycle hooks, with merged #3542 as the shipped consolidation. #70257 is complementary, not a duplicate, because it changes the separate memory-provider availability path.

Suggested consolidation

Author action: rebase #2768 onto main, or split out the part that can close the generic-loader gap, using #68703's pre-import str/dict validation and registry-backed regressions while retaining any complete registration-surface accounting from #58692. This preserves #2768's recorded best-fix designation and addresses its keep_open verdict; keep #68703 open as the concrete salvage path until that transfer is complete, then #58692 and #68703 can close as duplicates of #2768, while closed #70257 remains preserved through successor #74379 and the lifecycle PRs remain consolidated in merged #3542.

Complex graph

flowchart LR
    classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
    classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
    classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
    classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
    classDef best stroke-width:3px,stroke:#b45309
    classDef target stroke-width:3px,stroke:#4338ca
    I2765(["issue #2765 (open)"])
    subgraph Dup63050 ["PRs duplicating each other"]
        P63050["PR #63050 (closed)"]
        P68703["PR #68703 (open)"]
    end
    P68703 -.->|partial| I2765
    class I2765 open
    class P63050 closed
    class P68703 open
    class P68703 target
    click I2765 "https://github.com/NousResearch/hermes-agent/issues/2765"
    click P63050 "https://github.com/NousResearch/hermes-agent/pull/63050"
    click P68703 "https://github.com/NousResearch/hermes-agent/pull/68703"

Loading

Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).

Cross-PR triage: Reviewed 10 pull requests and 2 issues in this complex. Each diff was read against this issue; Assessment working set: 109 kB of PR diffs, 29 kB of issue/PR text, 20 kB of discussion (27 comments), 14 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.