fix(plugins): classify pip entry-point provider plugins without importing - #76567
fix(plugins): classify pip entry-point provider plugins without importing#76567m1k3s0 wants to merge 3 commits into
Conversation
…ting Entry-point (pip-installed) plugins exposing register_memory_provider() or register_provider() + ProviderProfile were treated as plain standalone plugins and eagerly imported by the general PluginManager, even though memory and model providers have their own discovery systems and the module has no register() for the general manager to call. The import registered nothing and paid the module's full import cost in every Hermes process (a pip memory provider pulls fastembed -> onnxruntime, ~60 MB RSS). Entry-point manifests now get the same source-scan classification as directory plugins via a shared _detect_kind_from_source() helper: the module is resolved with importlib.util.find_spec (no import) and its first 8192 chars are scanned for provider markers. Memory providers -> kind=exclusive, model providers -> kind=model-provider; both are recorded for introspection and skipped by the general loader. Unresolvable or non-Python modules stay standalone (default behavior unchanged). Tests: an enabled pip entry-point memory provider is never imported; a pip entry-point model provider routes to providers/ discovery.
pestoura
left a comment
pestoura
left a comment
There was a problem hiding this comment.
importlib.util.find_spec(module_name) does not preserve the stated no-import property for dotted entry points: resolving package.plugin imports package first. That means a provider whose heavy imports live in package/__init__.py still pays the startup cost this PR is intended to remove, and may execute arbitrary package initialization during discovery.
Please cover a dotted entry point with a parent __init__.py that records execution (or imports a sentinel dependency), then resolve the child module without importing the parent—for example by locating the distribution files/module path from entry-point metadata rather than calling find_spec() on the dotted name. The regression should assert neither the parent package nor child module enters sys.modules during classification. This is from source inspection; I did not execute the plugin suite locally.
find_spec() on a dotted module name imports the parent package first, executing its __init__.py — which is exactly where a provider's heavy imports typically live (fastembed -> onnxruntime and friends). The previous classifier only preserved the no-import property for top-level entry points. _resolve_module_source() now resolves only the top-level name with find_spec() (import-free for top-level names) and walks the remaining dotted segments through submodule_search_locations by hand, mirroring PathFinder's file conventions (part.py module / part/__init__.py package). Namespace packages, zipped modules, extension modules, and anything else unexpected fall back to standalone (the safe default). .pyc origins map back to source via source_from_cache. Regression: a dotted entry point whose parent __init__.py writes an execution marker and imports the child — asserts the parent never executed and neither module enters sys.modules during classification. Fails against the previous implementation (marker written), passes now.
m1k3s0
left a comment
m1k3s0
left a comment
There was a problem hiding this comment.
Good catch — the no-import property was broken for dotted entry points, and the docstring even hand-waved it. Fixed in 991cfd1.
_classify_entrypoint_kind now goes through a new _resolve_module_source(): the top-level name is resolved with find_spec() (import-free for top-level names), then the remaining dotted segments are walked by hand through submodule_search_locations (mirroring PathFinder: part.py module / part/__init__.py package). The parent package is never imported, so package/__init__.py initialization never runs during discovery. Namespace packages, zipped modules, extension modules, and anything else unexpected fall back to standalone (the safe default).
Added the regression you asked for: test_entrypoint_dotted_name_never_imports_parent_package — a dotted entry point mempalace_pkg.provider:register whose parent __init__.py writes an execution marker and imports the child. It asserts the marker never appears and that neither mempalace_pkg nor mempalace_pkg.provider enters sys.modules during classification. Verified red-green: against the previous implementation the test fails (parent executed, marker written); with the fix, passes. Also re-verified E2E through real importlib.metadata with a fake dotted dist whose parent writes a marker — parent never executed, neither module imported.
teknium1
left a comment
teknium1
left a comment
There was a problem hiding this comment.
Thanks for isolating the eager-import path and for adding the dotted-entry-point regression; the no-parent-import resolver addresses the earlier review concern.
Problems
hermes_cli/plugins.py:1801classifies matchinghermes_agent.pluginsentry points asexclusive/model-provider, so the general manager skips their import. But the destinations are directory-only today:plugins/memory/__init__.py:90-121enumerates provider directories, andproviders/__init__.py:140-190imports only bundled/user directories plus legacy modules. A pip-only provider is therefore recorded but cannot be activated.- The new model-provider test checks only non-import. It should also exercise
providers.get_provider_profile()(and memory-provider activation for the memory path) against a real entry point, so the routing contract is verified end to end.
Suggested changes
- Add specialized entry-point discovery/loading before routing pip providers away from the general manager, with activation and precedence tests; otherwise preserve the existing path for pip entry points.
Automated hermes-sweeper review.
| @@ -1694,6 +1801,7 @@ def _scan_entry_points(self) -> List[PluginManifest]: | |||
| path=ep.value, | |||
There was a problem hiding this comment.
This routes matching pip entry points into the exclusive/model-provider skip branches, but those loaders currently only scan filesystem directories (plugins/memory/__init__.py:90-121; providers/__init__.py:140-190). A pip-only provider will be recorded here yet never activate. Please add specialized entry-point discovery/loading and an activation test before assigning this kind.
There was a problem hiding this comment.
Verified both citations — the facts hold. The routing is still correct, and the contract is now pinned by tests (c8932b4).
"Recorded but cannot be activated" is the pre-existing state, not a regression: before this change a pip-only provider was also never activatable (memory discovery scans dirs only at plugins/memory/init.py:90-121, and the hermes_agent.memory_providers group those plugins actually declare has zero consumers in the tree; providers/init.py:140-190 is dirs + legacy files). It was just imported first at full cost (mnemosyne → fastembed → onnxruntime, +63 MB RSS per process) and logged no register() function. So "preserve the existing path" would preserve a path with no activation capability in it.
The measured bug is the duplicate shape, and the new test pins it end-to-end: pip entry point + same-name directory provider → pip copy classified exclusive and never imported, directory copy still activates via plugins.memory discovery exactly once (test_entrypoint_duplicate_does_not_block_directory_provider_activation).
Full entry-point activation belongs in the discovery systems, and #40644 already tracks it for memory. This change is its prerequisite — once entry-point discovery loads pip providers deliberately, classification is what prevents the same module from also being imported by the general manager. Landing the skip-import half now is a strict win in every environment; folding activation + precedence into this PR would duplicate #40644 scope.
Also per your ask: the model-provider test now exercises providers.get_provider_profile() against a real entry point (None today — directory-only contract — with the module still absent from sys.modules), and _classify_entrypoint_kind documents the activation contract in-source.
…ation Documents and tests the routing contract the sweeper review asked about: classification records the manifest but does not activate anything. - model-provider test now exercises providers.get_provider_profile() against the pip-only name (None today — providers discovery is directory-based) and asserts the module never leaks into sys.modules via that path. - new test for the mnemosyne shape: a pip entry point duplicating a same-name directory provider. The pip copy is classified exclusive and never imported; the directory copy still activates through plugins.memory discovery, exactly once. - _classify_entrypoint_kind docstring now states the activation contract explicitly: pip-only providers were equally unactivatable pre-change (both destination systems are directory-only; the hermes_agent.memory_providers entry-point group has no consumers), so classification only removes the wasted import. Entry-point activation is tracked upstream (NousResearch#40644 for memory); this change is its prerequisite, preventing double import once it lands.
teknium1
added
sweeper:risk-compatibility
alt-glitch
added
type/bug
…ee providers Builds on the three salvaged commits: adds the sources and integration points they leave out, so a pip-installed memory provider is not a second-class citizen next to a directory install. Discovery - Project-local providers (./.hermes/plugins/<name>/), gated on HERMES_ENABLE_PROJECT_PLUGINS exactly as PluginManager gates its own project scan. Completes the four sources CONTRIBUTING.md and AGENTS.md already promised; memory was the only discovery system missing two of them. - find_provider_dir() now resolves a package entry point to its directory. This is load-bearing: config_schema.py (the dashboard panel) and cli.py (the `hermes <provider>` subcommands) are read from disk rather than imported, so without a directory a pip-installed provider silently lost both. - list_memory_provider_names() includes entry-point providers, so they appear in the dashboard's memory.provider dropdown. Resolution stays import-free. hermes_cli.plugins.resolve_module_origin() is extracted from _resolve_module_source() (added by the salvaged NousResearch#76567) and shared, so discovery walks a module's file layout instead of importing it. find_provider_dir() is called from the dashboard and from argparse setup, long before the operator has chosen a provider — importing every installed candidate would execute third-party code on the strength of a package being present. A test asserts the resolution leaves no side effects and no sys.modules entry. Registration - PluginContext gains register_memory_provider(). Memory was the only provider category without one; context engine, image gen, video gen, web search, browser, TTS, transcription, secret source, dashboard auth and platform all have one. - _ProviderCollector delegates unknown register_* calls to a real PluginContext instead of carrying three hand-written no-ops. It silently dropped register_tool/register_hook, and had no register_auxiliary_task at all — despite PluginContext.register_auxiliary_task documenting a memory provider (hindsight's pre-retain dedup) as its worked example. It can no longer drift behind PluginContext. - A raise after register_memory_provider() no longer costs the provider. The loader caught it into a debug log, discarded the registered instance, and fell through to "instantiate any MemoryProvider subclass" — returning a different, unconfigured provider. A silent downgrade that looked like success, and the exact outcome of calling register_auxiliary_task. Activation is unchanged: still gated on memory.provider naming the plugin, and covered by a test so the real PluginContext cannot start requiring plugins.enabled — that would break every existing user-installed provider. Verified end to end against a real third-party provider (kainappsinc/elephant) installed by pip alone, with no directory copy: it appears in the dropdown, resolves its directory, loads with its tools, and renders its dashboard panel. Closes NousResearch#40101.
…ee providers Builds on the three salvaged commits: adds the sources and integration points they leave out, so a pip-installed memory provider is not a second-class citizen next to a directory install. Discovery - Project-local providers (./.hermes/plugins/<name>/), gated on HERMES_ENABLE_PROJECT_PLUGINS exactly as PluginManager gates its own project scan. Completes the four sources CONTRIBUTING.md and AGENTS.md already promised; memory was the only discovery system missing two of them. - find_provider_dir() now resolves a package entry point to its directory. This is load-bearing: config_schema.py (the dashboard panel) and cli.py (the `hermes <provider>` subcommands) are read from disk rather than imported, so without a directory a pip-installed provider silently lost both. - list_memory_provider_names() includes entry-point providers, so they appear in the dashboard's memory.provider dropdown. Resolution stays import-free. hermes_cli.plugins.resolve_module_origin() is extracted from _resolve_module_source() (added by the salvaged #76567) and shared, so discovery walks a module's file layout instead of importing it. find_provider_dir() is called from the dashboard and from argparse setup, long before the operator has chosen a provider — importing every installed candidate would execute third-party code on the strength of a package being present. A test asserts the resolution leaves no side effects and no sys.modules entry. Registration - PluginContext gains register_memory_provider(). Memory was the only provider category without one; context engine, image gen, video gen, web search, browser, TTS, transcription, secret source, dashboard auth and platform all have one. - _ProviderCollector delegates unknown register_* calls to a real PluginContext instead of carrying three hand-written no-ops. It silently dropped register_tool/register_hook, and had no register_auxiliary_task at all — despite PluginContext.register_auxiliary_task documenting a memory provider (hindsight's pre-retain dedup) as its worked example. It can no longer drift behind PluginContext. - A raise after register_memory_provider() no longer costs the provider. The loader caught it into a debug log, discarded the registered instance, and fell through to "instantiate any MemoryProvider subclass" — returning a different, unconfigured provider. A silent downgrade that looked like success, and the exact outcome of calling register_auxiliary_task. Activation is unchanged: still gated on memory.provider naming the plugin, and covered by a test so the real PluginContext cannot start requiring plugins.enabled — that would break every existing user-installed provider. Verified end to end against a real third-party provider (kainappsinc/elephant) installed by pip alone, with no directory copy: it appears in the dropdown, resolves its directory, loads with its tools, and renders its dashboard panel. Closes #40101.
|
Resolved via PR #85527 (salvage of #80493, which carried your commits) — your |
4 participants
What & why
Pip-installed plugins that declare a
hermes_agent.pluginsentry point but exposeregister_memory_provider()(memory providers) orregister_provider()+ProviderProfile(model providers) are treated as plainstandaloneplugins and eagerly imported by the general PluginManager — even though both categories have their own discovery systems and the module has noregister()for the general manager to call. The import registers nothing and only pays the module's full import cost in every Hermes process (CLI, gateway, desktop backend, cron, delegation children).Real-world cost (measured, 2026-08):
mnemosyne-hermesdeclares ahermes_agent.pluginsentry point and exposes onlyregister_memory_provider()(its real registration is thehermes_agent.memory_providerspath / its directory copy). Withmemory.providerunset (built-in), every process still importedmnemosyne_hermes → fastembed → onnxruntime: +63 MB RSS inimport run_agent, with the manager loggingPlugin 'mnemosyne' has no register() function, enabled=False. Nothing was registered — the cost was pure waste.Directory plugins already avoid this:
_parse_directory_manifestauto-coerceskind=exclusive/kind=model-providervia a cheap source scan. Entry-point manifests never got that classification — a systemic gap, since memory/model providers are exactly the plugins that declare dual entry points.What changed
hermes_cli/plugins.py— extracted the source-marker → kind detection from the directory-manifest path into a shared module-level helper_detect_kind_from_source()(behavior-identical; the directory path now calls it).hermes_cli/plugins.py— newPluginManager._classify_entrypoint_kind(): resolves the entry point's module viaimportlib.util.find_spec()(no import for top-level names; dotted names may import only the parent package), scans the first 8192 chars of source, and applies the same heuristic._scan_entry_points()now stamps the kind on each manifest.tests/hermes_cli/test_plugins.py— five tests: a pip entry-point memory provider is recorded asexclusiveand never imported (explicitsys.modulescheck), even when explicitly enabled; a pip entry-point model provider is routed tomodel-provider; a dotted entry point (pkg.mod:register) whose parent__init__.pyrecords execution is classified without the parent ever running or enteringsys.modules; the model-provider test additionally exercisesproviders.get_provider_profile()(None — directory-only routing contract, module still absent fromsys.modules); the mnemosyne shape (pip entry point duplicating a same-name directory provider) is classified exclusive while the directory copy still activates throughplugins.memorydiscovery exactly once.Design decisions
.pyorigins, and read failures staystandalone— no behavior change outside the two provider categories.register_memory_provider/MemoryProvider→exclusive;register_provider+ProviderProfile→model-provider).find_spec()(import-free for top-level names); dotted names are walked segment-by-segment throughsubmodule_search_locationsby hand, sopackage/__init__.pyinitialization (where heavy imports typically live) never runs during discovery. Unresolvable / namespace / zipped / extension modules fall back tostandalone.memory.providerviaplugins/memory;providers/lazy discovery). A pip-only provider is therefore recorded for introspection but not activatable until those systems gain entry-point discovery — the pre-existing state, not a regression: it was equally unactivatable before (thehermes_agent.memory_providersentry-point group has zero consumers; both destinations scan directories only), it merely paid the import cost first and loggedno register() function. Entry-point activation is tracked separately in feat: discover pipx-installed memory providers via entry points, switch to pipx #40644 (memory providers); this change is its prerequisite, preventing double-import once it lands. The contract is stated in_classify_entrypoint_kind's docstring.Testing
scripts/run_tests.sh tests/hermes_cli/test_plugins.py -q→ 39 passed, 0 failed (35 pre-existing + 4 new).module 'fakeprovider' has no attribute 'register'for the flat case; the parent-package execution marker written for the dotted case); with the fix, all pass.test_startup_plugin_gating.py,test_plugin_scanner_recursion.py,test_plugins_cmd_enable_disable_nested.py,test_plugin_cli_registration.py.entry_points.txtonPYTHONPATH, discovered through realimportlib.metadata→kind=exclusive, recorded, module absent fromsys.modules.Duplicate search
gh search issues/gh search prsfor: "entry point plugin import", "plugin exclusive memory provider", "memory provider plugin onnxruntime", "lazy plugin discovery", "register_memory_provider" — no PR implements entry-point kind classification. Adjacent open work, cited for context:MemoryProvider; complementary, no overlap.