diff --git a/hermes_cli/plugins_cmd_toggle.py b/hermes_cli/plugins_cmd_toggle.py index 20efb50674..02e9e613e0 100644 --- a/hermes_cli/plugins_cmd_toggle.py +++ b/hermes_cli/plugins_cmd_toggle.py @@ -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.`` 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.") diff --git a/tests/hermes_cli/test_plugins_admission_setter.py b/tests/hermes_cli/test_plugins_admission_setter.py index 4fbe3cb504..39a9a6c8ba 100644 --- a/tests/hermes_cli/test_plugins_admission_setter.py +++ b/tests/hermes_cli/test_plugins_admission_setter.py @@ -93,7 +93,8 @@ def test_ui_conflict_is_reported_without_changing_selection(plugin_world, surfac assert not result["ok"] and "plugin-proof-dep" in result["error"] else: with pytest.raises(AdmissionRefused, match="plugin-proof-dep"): - plugins_cmd._persist_plugin_selection(["plugin-worker-proof", "conflicting-ui-plugin"], {0, 1}, set()) + # plugin-worker-proof is ticked on open; ticking the conflicting plugin is the refused flip. + plugins_cmd._persist_plugin_selection(["plugin-worker-proof", "conflicting-ui-plugin"], {0, 1}, set(), {0}) assert {path: path.read_bytes() for path in watched} == before world.imports() diff --git a/tests/hermes_cli/test_plugins_cmd_enable_disable_nested.py b/tests/hermes_cli/test_plugins_cmd_enable_disable_nested.py index 3858f2223d..fa1f6908a1 100644 --- a/tests/hermes_cli/test_plugins_cmd_enable_disable_nested.py +++ b/tests/hermes_cli/test_plugins_cmd_enable_disable_nested.py @@ -30,9 +30,11 @@ def test_nested_enable_disable_and_composite_use_canonical_key(plugin_world, mon world.command("disable", name=query) assert world.enabled() == [] assert yaml.safe_load((world.home / "config.yaml").read_text())["plugins"]["disabled"] == [key] - # The fallback menu must persist canonical keys too, not labels. - monkeypatch.setattr("builtins.input", lambda prompt: "") - plugins_cmd._run_composite_fallback([key], ["trace-manifest"], {0}, {key}, [], Console()) + # The fallback menu must persist canonical keys too, not labels: the row opens unticked (disabled) + # and the user ticks it. + answers = iter(("1", "")) + monkeypatch.setattr("builtins.input", lambda prompt: next(answers)) + plugins_cmd._run_composite_fallback([key], ["trace-manifest"], set(), {key}, [], Console()) assert world.enabled() == [key] world.imports(key) diff --git a/website/docs/user-guide/features/plugins.md b/website/docs/user-guide/features/plugins.md index 875e3fa79b..e8623a0c7c 100644 --- a/website/docs/user-guide/features/plugins.md +++ b/website/docs/user-guide/features/plugins.md @@ -806,7 +806,7 @@ Plugins Context Engine ▸ compressor ``` -- **General Plugins section** — checkboxes, toggle with SPACE. Checked = in `plugins.enabled`, unchecked = in `plugins.disabled` (explicit off). +- **General Plugins section** — checkboxes, toggle with SPACE. A row opens checked when the plugin is active right now: listed in `plugins.enabled`, or a bundled platform, backend or model provider (on without a list entry), or the selected provider of a category. Only rows you flip are written on exit: unticking adds the plugin to `plugins.disabled` (explicit off), ticking adds it to `plugins.enabled` and clears a stale disable. Opening the picker and leaving changes nothing. - **Provider Plugins section** — shows current selection. Press ENTER to drill into a radio picker where you choose one active provider. - Bundled plugins appear in the same list with a `[bundled]` tag.