fix(plugins): picker preselects the effective state and persists only flipped rows

The bare `hermes plugins` picker preselected only rows listed in
plugins.enabled. Bundled platforms, backends and model providers are active
without a list entry, so they opened unticked, and the save on exit wrote every
unticked row into plugins.disabled: opening the picker and leaving without a
change disabled every messaging adapter on the next gateway restart.

Rows now open ticked by the load-time rule (_plugin_status), and only rows the
user flipped are written.

Ported onto the plugins_cmd_toggle sibling and the admission-authority save
(the original targeted the pre-split plugins_cmd.py). The nested
canonical-key composite test now ticks its row explicitly: it handed the menu
a checkbox state that contradicted the config and relied on the old
rebuild-everything save. The original helper-level tests are rewritten
against real admission in a follow-up commit.

(cherry picked from commit 3a0d5f1b8bce3b23b115ba4995b8c23dfbba9ad0)
This commit is contained in:
Victor Kyriazakos
2026-09-24 13:52:47 +00:00
committed by kshitij
parent c29114ea9a
commit bfb8d3bd64
4 changed files with 52 additions and 30 deletions

View File

@@ -8,8 +8,11 @@ imported late here, never at module level).
from __future__ import annotations
import functools
import logging
import sys
logger = logging.getLogger(__name__)
def _pc():
"""The facade, read at call time: tests patch ``plugins_cmd.<name>`` and sibling calls must see it."""
@@ -100,11 +103,7 @@ def cmd_toggle() -> None:
(f"{name} \u2014 {description}" if description else name) + (" [bundled]" if source == "bundled" else "")
for name, _version, description, source, _d, _key in entries
]
# Selected when enabled AND not disabled; the legacy bare name counts on either side.
plugin_selected = {
i for i, (name, _v, _desc, _src, _d, key) in enumerate(entries)
if {key, name} & enabled_set and not ({key, name} & disabled_set)
}
plugin_selected = _effective_plugin_selection(entries, enabled_set, disabled_set)
categories = _pc()._provider_categories()
if not sys.stdin.isatty():
@@ -117,33 +116,52 @@ def cmd_toggle() -> None:
_run_composite_fallback(plugin_keys, plugin_labels, plugin_selected, disabled_set, categories, console, expected_config=expected_config)
def _persist_plugin_selection(plugin_keys, chosen, disabled, *, expected_config=None) -> tuple[bool, set]:
def _effective_plugin_selection(entries, enabled: set, disabled: set) -> set:
"""Row indices ticked when the picker opens: every plugin that is ACTIVE right now, by the same
rule ``gate_manifest`` applies at load (``_plugin_status``), not merely the ones listed in
``plugins.enabled``. Bundled platforms, backends and model providers are on without a list
entry, so listing-based preselection showed them unticked and a no-change exit disabled every
messaging adapter on the next gateway restart."""
active = _pc()._category_active_names()
return {
i for i, (name, _v, _desc, source, dir_path, key) in enumerate(entries)
if _pc()._plugin_status(name, enabled, disabled, key, source=source, dir_path=dir_path,
active=active) == "enabled"
}
def _persist_plugin_selection(plugin_keys, chosen, disabled, initial, *, expected_config=None) -> tuple[bool, set]:
"""Save the composite UI's checkbox state; returns ``(changed, new_enabled)``.
Unchecked plugins go to the disabled-list (so they stay off even if something auto-enables
them) under the canonical key ONLY, so the list can't drift from what ``cmd_enable`` clears.
Re-checking also drops any stale legacy bare-leaf disable.
Only rows the user actually flipped are written: a plugin unticked in this session goes to
the disabled-list (so it stays off even if something auto-enables it) under the canonical key
ONLY, so the list can't drift from what ``cmd_enable`` clears; a plugin ticked in this session
is added to ``plugins.enabled`` and any stale legacy bare-leaf disable is dropped. Rows left
as they were are not persisted, so "never ticked" is never mistaken for "explicitly unticked"
and list entries for plugins not shown in the picker survive.
"""
# See #40190.
# Persist by canonical key only — never the bare manifest name — so the disabled-list stays aligned with
# cmd_enable / PluginManager (#40190).
if expected_config is None:
expected_config = _pc()._plugin_selection_version()
new_enabled: set = set()
new_disabled: set = set(disabled) # preserve existing disabled state for unseen plugins
for i, key in enumerate(plugin_keys):
if i in chosen:
new_enabled.add(key)
_pc()._discard_key_and_leaf(new_disabled, key)
else:
new_disabled.add(key)
new_enabled: set = set(_pc()._get_enabled_set())
new_disabled: set = set(disabled)
turned_on = [plugin_keys[i] for i in sorted(chosen - initial)]
turned_off = [plugin_keys[i] for i in sorted(initial - chosen)]
for key in turned_on:
new_enabled.add(key)
_pc()._discard_key_and_leaf(new_disabled, key)
for key in turned_off:
_pc()._discard_key_and_leaf(new_enabled, key)
new_disabled.add(key)
changed = new_enabled != _pc()._get_enabled_set() or new_disabled != disabled
changed = bool(turned_on or turned_off)
if changed:
# C13: the composite UI's candidate goes through the ONE admission
# authority — refusal raises AdmissionRefused BEFORE any config
# write; the caller surfaces it and the selection stays unsaved.
_pc()._admit_and_save_plugin_sets(new_enabled, new_disabled, action="Save plugin selection", expected_config=expected_config)
logger.info("plugins picker: enabled %s; disabled %s", turned_on or "none", turned_off or "none")
return changed, new_enabled
@@ -260,7 +278,8 @@ def _run_composite_ui(curses, plugin_keys, plugin_labels, plugin_selected, disab
from hermes_cli.plugins_admission import AdmissionRefused
try:
changed, new_enabled = _persist_plugin_selection(plugin_keys, chosen, disabled, expected_config=expected_config)
changed, _new_enabled = _persist_plugin_selection(plugin_keys, chosen, disabled, plugin_selected,
expected_config=expected_config)
except AdmissionRefused as exc:
console.print(f"[red]✗[/red] Plugin selection refused, not saved: {exc}")
console.print(
@@ -270,8 +289,8 @@ def _run_composite_ui(curses, plugin_keys, plugin_labels, plugin_selected, disab
return
if changed:
console.print(
f"\n[green]\u2713[/green] General plugins: {len(new_enabled)} enabled, "
f"{len(plugin_keys) - len(new_enabled)} disabled.")
f"\n[green]\u2713[/green] General plugins: {len(chosen)} enabled, "
f"{len(plugin_keys) - len(chosen)} disabled.")
elif n_plugins > 0:
console.print("\n[dim]General plugins unchanged.[/dim]")
if providers_changed:
@@ -306,7 +325,7 @@ def _run_composite_fallback(plugin_keys, plugin_labels, plugin_selected, disable
except (ValueError, KeyboardInterrupt, EOFError):
return
print()
_save_plugin_selection_fallback(plugin_keys, chosen, disabled, expected_config=expected_config)
_save_plugin_selection_fallback(plugin_keys, chosen, disabled, plugin_selected, expected_config=expected_config)
if categories:
print(color("\n Provider Plugins", Colors.YELLOW))
@@ -324,12 +343,12 @@ def _run_composite_fallback(plugin_keys, plugin_labels, plugin_selected, disable
print()
def _save_plugin_selection_fallback(plugin_keys, chosen, disabled, *, expected_config=None) -> None:
def _save_plugin_selection_fallback(plugin_keys, chosen, disabled, initial, *, expected_config=None) -> None:
"""The text fallback's save: same admission authority, refusal printed."""
from hermes_cli.plugins_admission import AdmissionRefused
try:
_persist_plugin_selection(plugin_keys, chosen, disabled, expected_config=expected_config)
_persist_plugin_selection(plugin_keys, chosen, disabled, initial, expected_config=expected_config)
except AdmissionRefused as exc:
print(f" Plugin selection refused, not saved: {exc}")
print(" config.yaml and the active environment are unchanged.")