fix(plugins): refuse enable/disable on passive plugin kinds with a pointer to the real switch - #59183
Open
sam7894604 wants to merge 2 commits into
Open
fix(plugins): refuse enable/disable on passive plugin kinds with a pointer to the real switch#59183sam7894604 wants to merge 2 commits into
sam7894604 wants to merge 2 commits into
Conversation
…o the real switch
`hermes plugins enable gemini` printed a green success message and wrote
model-providers/gemini into plugins.enabled — but nothing ever reads that
flag for model providers: the general loader explicitly skips
kind: model-provider (handled by providers/__init__.py's own discovery,
selected via `hermes model` / model.provider) and kind: exclusive
(activated via `<category>.provider`). Same for disable: the entry lands
in plugins.disabled, the loader records it for introspection, and the
provider registers anyway.
The success message misleads users into believing they switched a
provider on or off. Found in the wild: a config with
model-providers/gemini in BOTH plugins.enabled (as a stray string) and
plugins.disabled, while the gemini provider had been registered and
usable the whole time.
enable/disable now detect the manifest kind and print what actually
controls the plugin, changing nothing:
! model-providers/gemini is a model provider — it is not controlled by
plugins.enabled/disabled (providers register automatically at startup).
To use it: run `hermes model` and pick it, or set model.provider.
To stop using it: select a different provider; remove its API key.
Nothing was changed.
alt-glitch
added
type/bug
teknium1
reviewed
Jul 15, 2026
teknium1
left a comment
teknium1
left a comment
Contributor
There was a problem hiding this comment.
Thanks for addressing the misleading success path; the model-provider premise is verified on current main: cmd_enable writes the general allow-list (hermes_cli/plugins_cmd.py:829-862), while provider discovery imports model-provider directories without reading that list (providers/__init__.py:156-171).
Problems
- The new
_plugin_kind()only recognizes an explicit manifestkind(hermes_cli/plugins_cmd.py:835in this PR). Current bundled memory providers are excluded from_discover_all_plugins()(hermes_cli/plugins_cmd.py:1027-1034) and their manifests, includingplugins/memory/honcho/plugin.yaml:1-7, do not declarekind: exclusive. - User memory providers can be classified as exclusive by the existing
__init__.pyheuristic (hermes_cli/plugins.py:1600-1627), which this new parser also bypasses. The synthetic explicit-kind test does not cover either production path.
Suggested changes
- Route memory-provider detection through
plugins.memorydiscovery (and preserve the existing model-provider route), then add tests for the bundled and inferred-memory conventions.
Automated hermes-sweeper review.
| try: | ||
| import yaml | ||
|
|
||
| data = yaml.safe_load(mf.read_text(encoding="utf-8")) or {} |
Contributor
There was a problem hiding this comment.
This explicit manifest lookup does not match the effective kind rules. Bundled memory providers are excluded from _discover_all_plugins() and their manifests (for example plugins/memory/honcho/plugin.yaml) omit kind; user memory providers can instead be inferred as exclusive from __init__.py. Route memory detection through the category discovery path or share the existing effective-kind logic.
teknium1
added
sweeper:risk-compatibility
Address review: _plugin_kind() no longer re-reads the raw manifest `kind` and no longer misses bundled memory providers. - Reuse PluginManager._parse_manifest so the register_memory_provider / register_provider heuristic (for manifests without an explicit kind) is shared, instead of defaulting user providers to standalone. - Recognize memory providers through plugins.memory discover_memory_providers(): bundled providers (e.g. honcho) ship without kind: exclusive and are excluded from _discover_all_plugins(), so cmd_enable/cmd_disable now intercept them before the general resolver and show the passive-kind hint instead of a misleading 'not installed' error. - Add production-path tests: bundled memory provider via category discovery, and the heuristic user memory/model provider paths (the prior tests only covered synthetic explicit-kind manifests).
Contributor
Author
|
Thanks for the review — both points are addressed in the latest push.
No rebase needed — this is a single follow-up commit on top of the existing branch and it stays mergeable. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
hermes plugins enable/disable <name>silently did nothing for passive plugin kinds — kinds whose activation is controlled elsewhere, not by the enable/disable switch. The command returned success but nothing changed, leaving the user unsure why.Why
Passive plugin kinds (e.g. memory providers and other config-driven kinds) are switched on/off through their own config, not through
plugins enable/disable. Invoking enable/disable on them was a no-op with no feedback. This makes the command recognize passive kinds and refuse with a clear message pointing to the real switch, instead of a silent no-op that looks like it worked.Changes
hermes_cli/plugins_cmd.py— detect passive plugin kinds and refuse enable/disable with a pointer to the actual control.tests/hermes_cli/test_plugins_enable_passive_kinds.py— new coverage.Testing
pytest tests/hermes_cli/test_plugins_enable_passive_kinds.py -q-> 5 passed.Cleanly cherry-picked onto current
main; touches only the two files above (no unrelated changes).