From ffe2f6411c3cb3a1f9be33e9caa423d4ea062931 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Sat, 24 Oct 2026 19:47:32 -0700 Subject: [PATCH] fix(pm): an update that disables a plugin keeps memory.provider and retries it When an update had to disable a plugin it also cleared memory.provider, so the memory provider stayed lost even after the plugin was re-enabled, and nothing ever retried the plugin: one transient failure (a TLS proxy, a mirror outage, a since-fixed resolver bug) left it off for good. - The dependency union now honours plugins.disabled for memory.provider the same way the provider loader already does, so eviction no longer has to blank the selection. Re-enabling the plugin brings the provider back. - The eviction is recorded in the home's config under plugins.disabled_by_update (was it enabled, and why). Every later update retries recorded plugins after the enabled ones and undoes exactly the edit once a plugin fits again. - A user's own enable/disable/remove takes the decision back: PM's selection publication forgets the record for the plugin the user settled, and `hermes plugins disable` on an update-disabled plugin now writes that. - Spellings come from the plugin's own manifest instead of a plugin scan of the PM process's home, so a profile's plugins are matched in that profile. - A parked provider is reported, not re-installed: agent init tells the user once why external memory is off (naming the update's reason), and `hermes memory status` and `hermes doctor` show it instead of "plugin not found" or "All checks passed". Doctor lists every update-disabled plugin. --- agent/agent_init.py | 30 +-- hermes_cli/doctor.py | 4 +- hermes_cli/doctor_state.py | 23 +++ hermes_cli/memory_setup.py | 6 +- hermes_cli/plugins_admission.py | 3 + hermes_cli/plugins_cmd.py | 19 +- plugins/memory/__init__.py | 23 ++- pm/plugin_eviction.py | 214 ++++++++++++++------- pm/plugins_state.py | 37 +++- pm/publication.py | 2 + pm/workspace.py | 27 ++- tests/pm/test_plugin_eviction_config.py | 217 ---------------------- tests/pm/test_plugin_survival_contract.py | 45 ++++- 13 files changed, 327 insertions(+), 323 deletions(-) delete mode 100644 tests/pm/test_plugin_eviction_config.py diff --git a/agent/agent_init.py b/agent/agent_init.py index 525d193413..5ef25a3fea 100644 --- a/agent/agent_init.py +++ b/agent/agent_init.py @@ -63,6 +63,17 @@ def _unavailable_warning_key(name: str) -> tuple[str, str]: return hermes_home_key(get_hermes_home()), name +def _warn_once(name: str, message: str, say=None) -> None: + """Log *message* and hand it to the user's sink once per home and provider.""" + key = _unavailable_warning_key(name) + if key in _warned_unavailable_providers: + return + _warned_unavailable_providers.add(key) + logger.warning(message) + if say is not None: + say(message) + + def _warn_memory_provider_unavailable(name: str, reason: str = "", say=None) -> None: """Warn once per home and provider that a configured memory provider is unavailable. @@ -71,20 +82,13 @@ def _warn_memory_provider_unavailable(name: str, reason: str = "", say=None) -> can only reach the user here, so it is appended when present. *say* is the agent's user-facing sink: a log line alone leaves the user running without memory unaware. """ - key = _unavailable_warning_key(name) - if key in _warned_unavailable_providers: - return - _warned_unavailable_providers.add(key) - message = ( + _warn_once(name, ( f"⚠ Memory provider {name!r} is selected but reports unavailable — external memory " "is disabled for this session (built-in memory still works). Check the " "provider's credentials/config with 'hermes memory status'. Note: " "systemd/gateway services do not inherit ~/.hermes/.env automatically; set " f"any required variables in the service environment.{f' {reason}' if reason else ''}" - ) - logger.warning(message) - if say is not None: - say(message) + ), say=say) def _provider_default_routes(provider: str) -> set[str]: @@ -1348,10 +1352,14 @@ def _init_memory(agent, _agent_cfg, skip_memory, platform, memory_manager=None): _mem_provider_name = mem_config.get("provider", "") if mem_config else "" if not is_core_memory_provider(_mem_provider_name): from agent.memory_manager import MemoryManager as _MemoryManager - from plugins.memory import load_memory_provider as _load_mem + from plugins.memory import load_memory_provider as _load_mem, parked_provider_notice agent._memory_manager = _MemoryManager() _mp = _load_mem(_mem_provider_name) - if _mp is None: + _parked = _mp is None and parked_provider_notice(_mem_provider_name) + if _parked: + # Installed but turned off (by the user or by an update): say so, never re-install it. + _warn_once(_mem_provider_name, _parked, say=agent._emit_startup_warning) + elif _mp is None: # The provider left core for the catalog (or was never installed): fetch it once. from hermes_cli.memory_provider_migration import recover_at_startup if recover_at_startup(_mem_provider_name, say=agent._emit_startup_warning): diff --git a/hermes_cli/doctor.py b/hermes_cli/doctor.py index b2972b0566..43d3478c1f 100644 --- a/hermes_cli/doctor.py +++ b/hermes_cli/doctor.py @@ -54,6 +54,7 @@ from hermes_cli.doctor_state import ( _check_cron_store, _check_directory_structure, _check_memory_provider, + _check_plugins_disabled_by_update, _check_profiles, _check_skills_hub, _check_state_db, @@ -124,7 +125,8 @@ DOCTOR_CHECKS = ( ('External Tools', _check_git_and_rg), (None, _check_terminal_backend), (None, _check_node_and_browser), (None, _check_npm_audit), ('API Connectivity', _check_api_connectivity), ('Tool Availability', _check_tool_availability), ('Skills Hub', _check_skills_hub), - ('Memory Provider', _check_memory_provider), ('NeMo Relay Plugins', _check_relay_plugins), + ('Memory Provider', _check_memory_provider), (None, _check_plugins_disabled_by_update), + ('NeMo Relay Plugins', _check_relay_plugins), (None, _check_profiles), ) diff --git a/hermes_cli/doctor_state.py b/hermes_cli/doctor_state.py index b74ce1399b..003bda333d 100644 --- a/hermes_cli/doctor_state.py +++ b/hermes_cli/doctor_state.py @@ -626,6 +626,11 @@ def _check_memory_provider(should_fix: bool, f: Finding) -> None: if is_core_memory_provider(name): check_ok("Built-in memory active", "(no external provider configured — this is fine)") return + from plugins.memory import parked_provider_notice + if parked_provider_notice(name): + check_warn(f"{name} is turned off", "(plugins.disabled lists it, so built-in memory is used)") + f.manual_issues.append(f"Memory provider '{name}' is turned off — run: hermes plugins enable {name}") + return checker, missing_row, missing_issue, label = _MEMORY_PROVIDER_CHECKS.get(name, (None, None, None, name)) try: checker(f.issues) if checker else _memory_provider_generic(name) @@ -638,6 +643,24 @@ def _check_memory_provider(should_fix: bool, f: Finding) -> None: check_warn(f"{label} check failed", str(_e)) +@doctor_check("") # best-effort: an unreadable config is the config check's finding +def _check_plugins_disabled_by_update(should_fix: bool, f: Finding) -> None: + """Plugins an update turned off (pm.plugin_eviction): every later update retries them.""" + from hermes_cli.doctor import HERMES_HOME + from pm.plugins_state import RECORD, read_home_selection + plugins = (read_home_selection(HERMES_HOME) or {}).get("plugins") or {} + record, disabled = plugins.get(RECORD), set(plugins.get("disabled") or ()) + entries = {name: entry for name, entry in record.items() if name in disabled} if isinstance(record, dict) else {} + if not entries: + return + _section("Plugins Turned Off by an Update") + for name, entry in entries.items(): + reason = entry.get("reason", "") if isinstance(entry, dict) else "" + check_warn(f"{name} is off", f"({reason})" if reason else "") + f.manual_issues.append(f"Plugin '{name}' was turned off by an update; the next update retries it, " + f"or run: hermes plugins enable {name}") + + @doctor_check("") # best-effort: profile enumeration must never break doctor def _check_profiles(should_fix: bool, f: Finding) -> None: from hermes_constants import PROFILE_ID_RE diff --git a/hermes_cli/memory_setup.py b/hermes_cli/memory_setup.py index 702fd22c5a..b28a7440c2 100644 --- a/hermes_cli/memory_setup.py +++ b/hermes_cli/memory_setup.py @@ -403,7 +403,11 @@ def cmd_status(args) -> None: for key, val in display_config.items(): print(f" {key}: {val}") - if provider: + from plugins.memory import parked_provider_notice + parked = parked_provider_notice(provider_name) + if parked: + print(f"\n Plugin: turned off ✗\n {parked}") + elif provider: print("\n Plugin: installed ✓") if provider.is_available(): print(" Status: available ✓") diff --git a/hermes_cli/plugins_admission.py b/hermes_cli/plugins_admission.py index 0b2d42fbba..6ba8f3c361 100644 --- a/hermes_cli/plugins_admission.py +++ b/hermes_cli/plugins_admission.py @@ -55,11 +55,13 @@ def admit_plugin_set_change( extra_dirs: Iterable[Path] = (), expected_config: str | None = None, plugin: Optional[str] = None, + settled: Iterable[str] = (), ) -> None: """PM discovers and validates the proposed union under its install lock. No config or dependency selection is written by this application process. *plugin* names the plugin being admitted so a resolver conflict is reported against it (:class:`DependencyConflict`). + *settled* names the plugins the user just decided on, so an update never undoes that decision. """ from hermes_constants import get_hermes_home from pm.client import sync_venv @@ -70,6 +72,7 @@ def admit_plugin_set_change( sync_venv(explicit=True, plugins=Selection({ "home": str(home.resolve()), "enabled": sorted(candidate_enabled), "disabled": sorted(candidate_disabled), "extra_dirs": [str(Path(d).resolve()) for d in extra_dirs], + "settled": sorted(settled), **({"expected_config": expected_config} if expected_config is not None else {}), })) except Exception as exc: diff --git a/hermes_cli/plugins_cmd.py b/hermes_cli/plugins_cmd.py index d8c3533252..f70b3ee230 100644 --- a/hermes_cli/plugins_cmd.py +++ b/hermes_cli/plugins_cmd.py @@ -490,7 +490,7 @@ def _plugin_selection_version() -> str: def _admit_and_save_plugin_sets( enabled: set, disabled: set, *, extra_dirs=(), console=None, action: str = "enable", expected_config=None, - plugin: Optional[str] = None, + plugin: Optional[str] = None, settled=(), ) -> None: """ONE admission authority for proposed enabled/disabled sets (C13): the candidate union is resolved against the ACTIVE environment and @@ -506,7 +506,7 @@ def _admit_and_save_plugin_sets( try: admit_plugin_set_change( enabled, disabled, active_plugins_dir=_plugins_dir(), extra_dirs=extra_dirs, expected_config=expected_config, - plugin=plugin, + plugin=plugin, settled=settled, ) except DependencyConflict as exc: # `hermes pm install` cannot fix a conflict, so the retry hint below would mislead here. @@ -570,12 +570,22 @@ def _activate_key(key: str, *, enable: bool, console=None) -> bool: enabled, disabled = _get_enabled_set(), _get_disabled_set() aliases = _plugin_aliases(key) target, other = (enabled, disabled) if enable else (disabled, enabled) - if key in target and not (aliases & other): + # A plugin an update disabled is retried by the next update; disabling it yourself stops that. + if key in target and not (aliases & other) and not (not enable and _disabled_by_update(key, aliases)): return False _set_plugin_enabled(key, enable=enable, aliases=aliases, console=console) return True +def _disabled_by_update(key: str, aliases) -> bool: + """Whether an update disabled this plugin and will retry it (``pm.plugin_eviction`` records it).""" + from pm.plugins_state import RECORD + from pm.plugins_state import read_home_selection + + record = ((read_home_selection(get_hermes_home()) or {}).get("plugins") or {}).get(RECORD) + return isinstance(record, dict) and bool({key, *aliases} & set(record)) + + def _forget_plugin_config(aliases: set) -> dict[str, Any]: """Drop every trace of a removed plugin (its :func:`_plugin_aliases`, taken BEFORE the tree went) from config.yaml: allow/deny-list entries, ``plugins.entries.`` grants and a ``memory.provider`` @@ -633,7 +643,8 @@ def _set_plugin_enabled(name: str, *, enable: bool, aliases=(), console=None) -> _apply_activation(enabled, disabled, name, aliases, enable=enable) _admit_and_save_plugin_sets(enabled, disabled, console=console, action=f"{'Enable' if enable else 'Disable'} '{name}'", - expected_config=expected_config, plugin=name if enable else None) + expected_config=expected_config, plugin=name if enable else None, + settled=(name, *aliases)) def _apply_activation(enabled: set, disabled: set, key: str, aliases, *, enable: bool) -> None: diff --git a/plugins/memory/__init__.py b/plugins/memory/__init__.py index 8a645f6f2b..24627e3dee 100644 --- a/plugins/memory/__init__.py +++ b/plugins/memory/__init__.py @@ -226,10 +226,9 @@ def load_memory_provider(name: str, *, register_skills: Optional[bool] = None) - logger.debug("Memory provider '%s' not found in bundled, user plugins, or entry points", name) return None if provider_dir is not None and _explicitly_disabled(name, provider_dir): - # The Plugins hub / `hermes plugins disable` park a user-installed provider in - # ``plugins.disabled``; the loader must honour it or "disabled" is a lie in the UI. - logger.warning("Memory provider '%s' is disabled via plugins.disabled; run `hermes plugins enable %s` " - "or change memory.provider.", name, name) + # The Plugins hub / `hermes plugins disable` (or an update that could not install it) park a + # user-installed provider in ``plugins.disabled``; the loader must honour it or "disabled" is a lie. + logger.warning("%s", parked_provider_notice(name)) return None def _load(_dir): @@ -504,6 +503,22 @@ def _get_active_memory_provider() -> Optional[str]: return None +def parked_provider_notice(name: str) -> Optional[str]: + """The user-facing reason a configured provider is not loading because ``plugins.disabled`` parks + it, naming the update that did so when there was one; None when it is not parked.""" + provider_dir = find_provider_dir(name) + if provider_dir is None or not _explicitly_disabled(name, provider_dir): + return None + from hermes_cli.config import load_config + from pm.plugins_state import RECORD + record = cfg_get(load_config(), "plugins", RECORD) + recorded = record.get(name) if isinstance(record, dict) else None + why = (f" An update turned it off ({recorded.get('reason')}) and the next update retries it." + if isinstance(recorded, dict) and recorded.get("reason") else "") + return (f"⚠ Memory provider '{name}' is turned off: plugins.disabled lists it, so external memory is off " + f"(built-in memory still works).{why} Turn it back on with `hermes plugins enable {name}`.") + + def _explicitly_disabled(name: str, provider_dir: Path) -> bool: """True when a NON-bundled provider is parked in ``plugins.disabled`` under any spelling the plugin commands write: the provider name, its directory name or its manifest ``name:``.""" diff --git a/pm/plugin_eviction.py b/pm/plugin_eviction.py index 33187794b6..5cba16e529 100644 --- a/pm/plugin_eviction.py +++ b/pm/plugin_eviction.py @@ -5,7 +5,9 @@ choose again. An update has nobody to ask and must never fail because of a plugi core moved (a newer Python, a bumped pin, a newer manifest contract) under a plugin that was admitted against the old core. Such a plugin is disabled in every home that enables it, the reason reaches the operator and the receipt, and the update continues -with the rest. Only a core that cannot build on its own still fails. +with the rest. Only a core that cannot build on its own still fails. The edit is recorded +in the home's config, so every later update retries the plugin and undoes the edit once it +fits again; a user who re-enables, disables or removes it takes the decision back. Disabling needs evidence about the plugin itself: its requires-python against the pinned interpreter, its manifest contract, a resolver proof, or its build failing. A fetch or @@ -21,7 +23,6 @@ import base64 import hashlib import io import json -import logging import subprocess import sys from pathlib import Path @@ -30,8 +31,7 @@ from pm.environment import BuildFailure, ResolutionConflict from pm.environments import install_state_dir, runtime_facts_path from pm.filesystem import durable_write_bytes, file_digest, native, read_bytes_or_none from pm.package import InstallError - -logger = logging.getLogger(__name__) +from pm.plugins_state import RECORD Entry = tuple[Path, str, Path] # (home plugins dir, selection key, plugin dir) @@ -77,77 +77,133 @@ def static_verdicts(entries: list[Entry], python_version: str) -> tuple[dict[Pat return reasons, waiting -def _discover_plugins() -> list | None: - """One scan of the plugin tree for alias lookup, or None when it cannot be read.""" - try: - from hermes_cli.plugins_cmd import _discover_all_plugins - - return _discover_all_plugins() - except (ImportError, OSError) as exc: - # Discovery only adds manifest-name aliases; the key and its bare leaf are always known. - # Anything else is a real bug in the plugin tree and should surface, not be hidden. - logger.warning("Could not discover plugin aliases: %s", exc) - return None - - -def _spellings(name: str, discovered: list | None) -> set[str]: - """Every spelling a config list may hold for plugin *name* (key, bare leaf, manifest name). - *discovered* is the shared scan from :func:`_discover_plugins`; None means the scan failed.""" - if discovered is None: - return {name, name.split("/")[-1]} - from hermes_cli.plugins_cmd import _plugin_aliases - - return _plugin_aliases(name, discovered) - - -def _discard_aliases(names: list, aliases: set[str]) -> None: - """Remove *aliases* from the round-tripped list in place, so order and comments survive.""" +def _discard(names: list, spellings: set[str]) -> None: + """Remove *spellings* from a round-tripped list in place, so order and comments survive.""" for index in reversed(range(len(names))): - if names[index] in aliases: + if names[index] in spellings: del names[index] -class PluginEviction: - """Config edits disabling the plugins in *reasons*; published like a plugin selection.""" +def _list(plugins, key: str) -> list: + if plugins.get(key) is None: + plugins[key] = [] + return plugins[key] - def __init__(self, entries: list[Entry], reasons: dict[Path, str]): + +def _disable(plugins, name: str, plugin_dir: Path, reason: str) -> None: + """Park *name* in plugins.disabled, drop it from plugins.enabled and record what to undo.""" + from pm.plugins_state import plugin_spellings + + spellings = plugin_spellings(plugin_dir, name) + disabled, enabled = _list(plugins, "disabled"), plugins.get("enabled") + if not isinstance(plugins.get(RECORD), dict): + plugins[RECORD] = {} + earlier = plugins[RECORD].get(name) + if name in disabled and isinstance(earlier, dict): + was_enabled = bool(earlier.get("enabled")) # a retry that still fails keeps the first record + else: + was_enabled = isinstance(enabled, list) and bool(spellings & set(enabled)) + if name not in disabled: + disabled.append(name) + if isinstance(enabled, list): + _discard(enabled, spellings) + plugins[RECORD][name] = {"enabled": was_enabled, "reason": " ".join(reason.split())[-300:]} + + +def _rejoin(plugins, name: str, plugin_dir: Path) -> None: + """Undo an earlier update's _disable now that the plugin fits again.""" + from pm.plugins_state import plugin_spellings + + record = plugins.get(RECORD) if isinstance(plugins.get(RECORD), dict) else {} + earlier = record.get(name) + _discard(_list(plugins, "disabled"), plugin_spellings(plugin_dir, name)) + if isinstance(earlier, dict) and earlier.get("enabled") and name not in _list(plugins, "enabled"): + plugins["enabled"].append(name) + + +def settle_record(plugins, settled=()) -> None: + """A user's own selection takes the decision back: forget the record of every plugin they just + enabled, disabled or removed (*settled*), so no later update turns it back on.""" + record = plugins.get(RECORD) + for name in settled if isinstance(record, dict) else (): + record.pop(name, None) + _prune_record(plugins) + + +def _prune_record(plugins) -> None: + """Forget records whose plugin left plugins.disabled (the user re-enabled or removed it).""" + record = plugins.get(RECORD) + if not isinstance(record, dict): + plugins.pop(RECORD, None) + return + disabled = set(plugins.get("disabled") or ()) + for name in [name for name in record if name not in disabled]: + del record[name] + if not record: + del plugins[RECORD] + + +def recorded_evictions() -> list[Entry]: + """Plugins an earlier update disabled that their home still has disabled: retried each update.""" + from pm.plugins_state import dependency_homes, read_home_selection + from pm.workspace import selected_plugin_dir + + entries: list[Entry] = [] + for home in dependency_homes(): + try: + plugins = (read_home_selection(home) or {}).get("plugins") or {} + except ValueError: + continue # sync_evicting already reports an unreadable profile + record, disabled = plugins.get(RECORD), plugins.get("disabled") or [] + for name in record if isinstance(record, dict) else (): + if not isinstance(name, str) or name not in disabled: + continue + try: + plugin_dir = selected_plugin_dir(home / "plugins", name) + except InstallError: + continue # a hand-edited key that names no plugin tree + if plugin_dir is not None: + entries.append((home / "plugins", name, plugin_dir)) + return entries + + +class PluginEviction: + """One update's config edits: disable the plugins in *reasons*, re-enable *rejoined* ones. + + A disabled plugin leaves plugins.enabled under every spelling and is recorded under + plugins.disabled_by_update (was it enabled, and why), so a later update can retry it and undo + exactly this edit. memory.provider is left alone: plugins.disabled keeps a parked provider out + of the environment and the loader, and re-enabling the plugin brings the provider back with it. + Published like a plugin selection. + """ + + def __init__(self, entries: list[Entry], reasons: dict[Path, str], rejoined: list[Entry] = ()): from hermes_yaml import roundtrip_yaml from pm.publication import selection_snapshot self.configs = selection_snapshot() - by_home: dict[Path, list[str]] = {} + by_home: dict[Path, list[tuple[str, Path, str | None]]] = {} for plugins_dir, name, plugin_dir in entries: if plugin_dir.resolve() in reasons: - by_home.setdefault(plugins_dir.parent, []).append(name) + by_home.setdefault(plugins_dir.parent, []).append((name, plugin_dir, reasons[plugin_dir.resolve()])) + for plugins_dir, name, plugin_dir in rejoined: + by_home.setdefault(plugins_dir.parent, []).append((name, plugin_dir, None)) self.edits: list[tuple[Path, bytes | None, bytes]] = [] - discovered: list | None = None - scanned = False # scan once for every evicted plugin, and only if an enabled list needs it - for home, names in by_home.items(): + for home, changes in by_home.items(): path = home / "config.yaml" previous = read_bytes_or_none(path) yaml = roundtrip_yaml() config = (yaml.load(previous.decode("utf-8-sig")) if previous else None) or {} - # read_home_selection already proved plugins/memory are mappings and the lists are lists. + # read_home_selection already proved plugins is a mapping and the lists are lists. plugins = config.get("plugins") if plugins is None: plugins = config["plugins"] = {} - disabled = plugins.get("disabled") - if disabled is None: - disabled = plugins["disabled"] = [] - enabled = plugins.get("enabled") - memory = config.get("memory") - for name in names: - if name not in disabled: - disabled.append(name) - # Keep the two lists consistent: a plugin that is disabled must not stay enabled - # under any spelling the loader matches, or `plugins enable` has nothing to repair. - if isinstance(enabled, list): - if not scanned: - discovered, scanned = _discover_plugins(), True - _discard_aliases(enabled, _spellings(name, discovered)) - # plugins.disabled does not veto memory.provider; the provider joins the union on its own. - if isinstance(memory, dict) and str(memory.get("provider") or "").strip() == name: - memory["provider"] = "" + for name, plugin_dir, reason in changes: + if reason is None: + _rejoin(plugins, name, plugin_dir) + else: + _disable(plugins, name, plugin_dir, reason) + _prune_record(plugins) output = io.StringIO() yaml.dump(config, output) self.edits.append((path, previous, output.getvalue().encode("utf-8"))) @@ -187,7 +243,8 @@ def sync_evicting(package, facts, fact: dict, *, extras, shipped, frozen, explic Static misfits go first (no resolver needed). If the rest still fails, core alone is built to prove the plugins are the cause, then members are re-added in config order - and each one that breaks the build is disabled too. + and each one that breaks the build is disabled too. Plugins an earlier update disabled + are retried after the enabled ones and re-enabled when they fit. """ from pm import receipt from pm.install import _commit_selection, _runtime_state_matches, _target_selection @@ -201,19 +258,23 @@ def sync_evicting(package, facts, fact: dict, *, extras, shipped, frozen, explic except ValueError as exc: notices.append(f"Skipped the plugins of profile {home}: {exc}; they rejoin once its config.yaml is fixed") entries = enabled_plugin_entries(skip_invalid_secondary=True) - reasons, waiting = static_verdicts(entries, _interpreter_version()) + retrying = recorded_evictions() + reasons, waiting = static_verdicts([*entries, *retrying], _interpreter_version()) + + def fits(plugin_dir: Path) -> bool: + return plugin_dir.resolve() not in reasons and plugin_dir.resolve() not in waiting def members() -> list[Path]: - return list(dict.fromkeys(plugin_dir for _plugins_dir, _name, plugin_dir in entries - if plugin_dir.resolve() not in reasons and plugin_dir.resolve() not in waiting - and _is_member_candidate(plugin_dir))) + return list(dict.fromkeys(plugin_dir for _plugins_dir, _name, plugin_dir in [*entries, *retrying] + if fits(plugin_dir) and _is_member_candidate(plugin_dir))) def commit() -> None: enabled, stamp, inputs = _target_selection(package, fact, extras=extras, inputs={"plugin_dirs": members()}, repair=False, shipped=shipped, frozen=frozen) receipt.record_feature_list(enabled) - _commit_selection(package, facts, PluginEviction(entries, reasons) if reasons else None, - enabled=enabled, stamp=stamp, inputs=inputs, + rejoined = [entry for entry in retrying if fits(entry[2])] + edit = PluginEviction([*entries, *retrying], reasons, rejoined) if reasons or rejoined else None + _commit_selection(package, facts, edit, enabled=enabled, stamp=stamp, inputs=inputs, current=_runtime_state_matches(fact, stamp), repair=False, explicit=explicit, skip_invalid_secondary=True) @@ -237,14 +298,25 @@ def sync_evicting(package, facts, fact: dict, *, extras, shipped, frozen, explic else: fitting.append(member) commit() - for plugins_dir, name, plugin_dir in entries: - key = plugin_dir.resolve() - if key in reasons: - notices.append(f"Disabled plugin '{name}' in {plugins_dir.parent}: {reasons[key]}; " - f"once it fits this Hermes again, run `hermes plugins enable {name}`") - elif key in waiting: - notices.append(f"Left plugin '{name}' in {plugins_dir.parent} out of this update: {waiting[key]}; " - "it stays enabled and rejoins once Hermes reports a version it accepts") + notices += _outcome_notices(entries, retrying, reasons, waiting) for message in notices: print(f"⚠ {message}", file=sys.stderr, flush=True) receipt.record_warning(message) + + +def _outcome_notices(entries: list[Entry], retrying: list[Entry], reasons: dict[Path, str], + waiting: dict[Path, str]) -> list[str]: + notices = [] + for plugins_dir, name, plugin_dir in entries: + key = plugin_dir.resolve() + if key in reasons: + notices.append(f"Disabled plugin '{name}' in {plugins_dir.parent}: {reasons[key]}; every update retries " + f"it and turns it back on once it fits, or run `hermes plugins enable {name}` sooner") + elif key in waiting: + notices.append(f"Left plugin '{name}' in {plugins_dir.parent} out of this update: {waiting[key]}; " + "it stays enabled and rejoins once Hermes reports a version it accepts") + for plugins_dir, name, plugin_dir in retrying: + why = reasons.get(plugin_dir.resolve()) or waiting.get(plugin_dir.resolve()) + notices.append(f"Plugin '{name}' in {plugins_dir.parent} is still disabled: {why}; the next update retries it" + if why else f"Turned plugin '{name}' in {plugins_dir.parent} back on: it fits this Hermes again") + return notices diff --git a/pm/plugins_state.py b/pm/plugins_state.py index ca1e4f9fb0..e58242352d 100644 --- a/pm/plugins_state.py +++ b/pm/plugins_state.py @@ -11,6 +11,9 @@ import logging LOG = logging.getLogger(__name__) +# plugins. maps each plugin an update disabled to {enabled, reason}: what to undo once it fits. +RECORD = "disabled_by_update" + def _profiles_root() -> Path: # Plugin discovery and dependency publication must use the same home root. @@ -151,10 +154,38 @@ def enabled_plugins_ordered(*, proposed_home=None, enabled=None, disabled=None, def _provider_from_config(home: Path, config: dict[str, Any], *, installing: Path | None = None) -> Optional[str]: """The ``memory.provider`` key of an already-parsed config, when its - plugin dir exists (no dir = not a member).""" + plugin dir exists (no dir = not a member) and plugins.disabled does not park it.""" provider = (config.get("memory") or {}).get("provider") if not provider or not provider.strip(): return None name = provider.strip() - return name if (_is_directory(home / "plugins" / name) or - (installing is not None and (home / "plugins" / name).resolve() == installing.resolve())) else None + plugin_dir = home / "plugins" / name + if installing is not None and plugin_dir.resolve() == installing.resolve(): + return name + if not _is_directory(plugin_dir) or _parked(plugin_dir, config): + return None + return name + + +def _parked(plugin_dir: Path, config: dict[str, Any]) -> bool: + """Whether plugins.disabled names this provider plugin. The provider loader honours the same + spellings (plugins.memory._explicitly_disabled), so a parked provider is never loaded and + installing its dependencies could only fail the generation for nothing.""" + disabled = (config.get("plugins") or {}).get("disabled") or [] + return bool(plugin_spellings(plugin_dir) & set(disabled)) + + +def plugin_spellings(plugin_dir: Path, key: str | None = None) -> set[str]: + """Every name a plugins list may use for this plugin: the selection key and its bare leaf, the + directory name and the manifest ``name:``. Read from the plugin itself, so it holds in any home.""" + import hermes_yaml + from ruamel.yaml.error import YAMLError + + names = {plugin_dir.name} | ({key, key.rsplit("/", 1)[-1]} if key else set()) + try: + manifest = hermes_yaml.safe_load((plugin_dir / "plugin.yaml").read_text(encoding="utf-8-sig")) or {} + names.add(str(manifest.get("name") or "")) + except (OSError, UnicodeError, YAMLError, AttributeError): + pass # no readable manifest: the other spellings still hold + names.discard("") + return names diff --git a/pm/publication.py b/pm/publication.py index 3ea1c7d4ac..b1d5db6def 100644 --- a/pm/publication.py +++ b/pm/publication.py @@ -62,6 +62,7 @@ def validate_manifest(source: Path) -> dict: class PluginSelection: def __init__(self, selection: dict): from hermes_yaml import roundtrip_yaml + from pm.plugin_eviction import settle_record self.configs = selection_snapshot() self.home = Path(selection["home"]).resolve() @@ -84,6 +85,7 @@ class PluginSelection: raise ValueError(f"plugins must be a mapping in {self.path}") plugins["enabled"] = sorted(selection["enabled"]) plugins["disabled"] = sorted(selection["disabled"]) + settle_record(plugins, selection.get("settled", ())) output = io.StringIO() yaml.dump(config, output) self.proposed = output.getvalue().encode("utf-8") diff --git a/pm/workspace.py b/pm/workspace.py index da375d07ed..6441a57b1c 100644 --- a/pm/workspace.py +++ b/pm/workspace.py @@ -217,7 +217,7 @@ def enabled_plugin_entries(*, proposed_home=None, enabled=None, disabled=None, The key is what the home's config names, so a caller can edit that home's selection. """ - from pm.plugins_state import _is_directory, enabled_plugins_ordered + from pm.plugins_state import enabled_plugins_ordered selection = enabled_plugins_ordered( proposed_home=proposed_home, enabled=enabled, disabled=disabled, installing=installing, @@ -226,18 +226,27 @@ def enabled_plugin_entries(*, proposed_home=None, enabled=None, disabled=None, entries = [] for plugins_dir, names in selection.items(): for name in names: - relative = Path(name) - if relative.is_absolute() or ".." in relative.parts: - raise InstallError("venv", f"invalid plugin key: {name}") - plugin_dir = plugins_dir / relative - proposed = installing is not None and plugin_dir.resolve() == installing.resolve() - if not proposed and not _is_directory(plugin_dir): - plugin_dir = paths.repo_root() / "plugins" / relative - if proposed or _is_directory(plugin_dir): + plugin_dir = selected_plugin_dir(plugins_dir, name, installing=installing) + if plugin_dir is not None: entries.append((plugins_dir, name, plugin_dir)) return entries +def selected_plugin_dir(plugins_dir: Path, name: str, *, installing: Path | None = None) -> Path | None: + """The tree a home's selection key names: its own plugins dir first, then core's, else None.""" + from pm.plugins_state import _is_directory + + relative = Path(name) + if relative.is_absolute() or ".." in relative.parts: + raise InstallError("venv", f"invalid plugin key: {name}") + plugin_dir = plugins_dir / relative + if installing is not None and plugin_dir.resolve() == installing.resolve(): + return plugin_dir + if not _is_directory(plugin_dir): + plugin_dir = paths.repo_root() / "plugins" / relative + return plugin_dir if _is_directory(plugin_dir) else None + + def enabled_plugin_dirs(*, proposed_home=None, enabled=None, disabled=None, installing: Path | None = None, skip_invalid_secondary: bool = False) -> list[Path]: """Resolve the effective plugin selection without filtering dependency declarations.""" diff --git a/tests/pm/test_plugin_eviction_config.py b/tests/pm/test_plugin_eviction_config.py deleted file mode 100644 index 6b70daf094..0000000000 --- a/tests/pm/test_plugin_eviction_config.py +++ /dev/null @@ -1,217 +0,0 @@ -"""Config edits made by PluginEviction keep plugins.enabled and plugins.disabled consistent.""" - -from __future__ import annotations - -import hashlib -import json -from pathlib import Path - -import pytest - -import hermes_yaml as yaml -import hermes_cli.plugins_cmd as _plugins_cmd -import pm.publication as publication -from pm.plugin_eviction import PluginEviction, _discard_aliases - - -_REAL_ALIASES = _plugins_cmd._plugin_aliases -_REAL_DISCOVER = _plugins_cmd._discover_all_plugins - - -@pytest.fixture(autouse=True) -def _no_alias_discovery(monkeypatch): - """Alias discovery scans installed plugins; these tests only need key + bare leaf.""" - import hermes_cli.plugins_cmd as plugins_cmd - - monkeypatch.setattr(plugins_cmd, "_discover_all_plugins", lambda: []) - monkeypatch.setattr(plugins_cmd, "_plugin_aliases", lambda key, entries=None: {key, key.split("/")[-1]}) - monkeypatch.setattr(publication, "selection_snapshot", lambda: {}) - - -def _entry(home: Path, name: str) -> tuple[Path, str, Path]: - plugin_dir = home / "plugins" / name - plugin_dir.mkdir(parents=True, exist_ok=True) - return home / "plugins", name, plugin_dir - - -def _evict(home: Path, names: list[str], text: str | bytes | None) -> dict: - config = home / "config.yaml" - if text is not None: - config.write_bytes(text if isinstance(text, bytes) else text.encode("utf-8")) - entries = [_entry(home, name) for name in names] - reasons = {plugin_dir.resolve(): "too old" for _d, _n, plugin_dir in entries} - eviction = PluginEviction(entries, reasons) - (_path, _previous, proposed), = eviction.edits - return yaml.safe_load(proposed.decode("utf-8")), proposed.decode("utf-8") - - -def test_evicted_plugin_leaves_enabled_and_joins_disabled(tmp_path): - cfg, _ = _evict(tmp_path, ["plug-b"], "plugins:\n enabled: [plug-a, plug-b]\n") - assert cfg["plugins"]["enabled"] == ["plug-a"] - assert cfg["plugins"]["disabled"] == ["plug-b"] - - -def test_only_enabled_case_creates_disabled_list(tmp_path): - cfg, _ = _evict(tmp_path, ["plug-a"], "plugins:\n enabled: [plug-a]\n") - assert cfg["plugins"]["enabled"] == [] - assert cfg["plugins"]["disabled"] == ["plug-a"] - - -def test_already_in_both_lists_is_repaired_without_duplicates(tmp_path): - cfg, _ = _evict(tmp_path, ["plug-a"], "plugins:\n enabled: [plug-a, plug-b]\n disabled: [plug-a]\n") - assert cfg["plugins"]["enabled"] == ["plug-b"] - assert cfg["plugins"]["disabled"] == ["plug-a"] - - -def test_missing_enabled_list_is_not_created(tmp_path): - cfg, _ = _evict(tmp_path, ["plug-a"], "plugins:\n disabled: [other]\n") - assert "enabled" not in cfg["plugins"] - assert cfg["plugins"]["disabled"] == ["other", "plug-a"] - - -def test_namespaced_and_bare_spellings_are_both_removed(tmp_path): - cfg, _ = _evict(tmp_path, ["web/firecrawl"], "plugins:\n enabled: [web/firecrawl, firecrawl, keep]\n") - assert cfg["plugins"]["enabled"] == ["keep"] - assert cfg["plugins"]["disabled"] == ["web/firecrawl"] - - -def test_comments_and_order_survive(tmp_path): - text = "# operator note\nplugins:\n enabled:\n - first # keep me\n - plug-b\n - last\n" - cfg, out = _evict(tmp_path, ["plug-b"], text) - assert "# operator note" in out - assert "# keep me" in out - assert cfg["plugins"]["enabled"] == ["first", "last"] - - -def test_running_twice_is_idempotent(tmp_path): - _cfg, first = _evict(tmp_path, ["plug-b"], "plugins:\n enabled: [plug-a, plug-b]\n") - cfg, second = _evict(tmp_path, ["plug-b"], first) - assert second == first - assert cfg["plugins"]["disabled"] == ["plug-b"] - - -def test_memory_provider_cleared_only_for_evicted_plugin(tmp_path): - cfg, _ = _evict(tmp_path, ["mem"], "plugins:\n enabled: [mem]\nmemory:\n provider: mem\n") - assert cfg["memory"]["provider"] == "" - other = tmp_path / "other" - other.mkdir() - cfg, _ = _evict(other, ["plug-a"], "plugins:\n enabled: [plug-a]\nmemory:\n provider: mem\n") - assert cfg["memory"]["provider"] == "mem" - - -def test_utf8_bom_config_round_trips(tmp_path): - cfg, out = _evict(tmp_path, ["plug-b"], "\ufeffplugins:\n enabled: [plug-a, plug-b]\n".encode("utf-8")) - assert cfg["plugins"]["enabled"] == ["plug-a"] - assert not out.startswith("\ufeff") - - -def test_secondary_home_is_edited_independently(tmp_path): - root, profile = tmp_path / "root", tmp_path / "root" / "profiles" / "work" - root.mkdir(parents=True) - profile.mkdir(parents=True) - (root / "config.yaml").write_text("plugins:\n enabled: [plug-a]\n", encoding="utf-8") - (profile / "config.yaml").write_text("plugins:\n enabled: [plug-a, keep]\n", encoding="utf-8") - entries = [_entry(root, "plug-a"), _entry(profile, "plug-a")] - eviction = PluginEviction(entries, {entries[1][2].resolve(): "too old"}) - (path, _previous, proposed), = eviction.edits - assert path == profile / "config.yaml" - assert yaml.safe_load(proposed.decode())["plugins"]["enabled"] == ["keep"] - - -def test_publish_records_hashes_and_writes_configs(tmp_path, monkeypatch): - import pm.plugin_eviction as module - - home = tmp_path / "home" - home.mkdir() - config = home / "config.yaml" - config.write_text("plugins:\n enabled: [plug-a]\n", encoding="utf-8") - entry = _entry(home, "plug-a") - eviction = PluginEviction([entry], {entry[2].resolve(): "too old"}) - state = tmp_path / "state" - state.mkdir() - monkeypatch.setattr(module, "install_state_dir", lambda project: state) - monkeypatch.setattr(module, "runtime_facts_path", lambda project: tmp_path / "facts.json") - eviction.publish(tmp_path) - - row = json.loads((state / "publication.json").read_text()) - (_p, previous, proposed), = eviction.edits - assert row["configs"][0]["config_after"] == hashlib.sha256(proposed).hexdigest() - assert config.read_bytes() == proposed - assert yaml.safe_load(config.read_text())["plugins"]["disabled"] == ["plug-a"] - - -def test_publish_refuses_when_selection_changed(tmp_path, monkeypatch): - home = tmp_path / "home" - home.mkdir() - (home / "config.yaml").write_text("plugins:\n enabled: [plug-a]\n", encoding="utf-8") - entry = _entry(home, "plug-a") - eviction = PluginEviction([entry], {entry[2].resolve(): "too old"}) - monkeypatch.setattr(publication, "selection_snapshot", lambda: {"changed": True}) - with pytest.raises(ValueError, match="changed while preparing"): - eviction.publish(tmp_path) - assert yaml.safe_load((home / "config.yaml").read_text())["plugins"]["enabled"] == ["plug-a"] - - -def test_discard_aliases_keeps_other_entries_in_order(): - names = ["a", "x", "b", "x"] - _discard_aliases(names, {"x"}) - assert names == ["a", "b"] - - -def test_alias_discovery_failure_from_io_falls_back_and_is_logged(tmp_path, monkeypatch, caplog): - import hermes_cli.plugins_cmd as plugins_cmd - - def unreadable(): - raise OSError("tree unreadable") - - monkeypatch.setattr(plugins_cmd, "_discover_all_plugins", unreadable) - with caplog.at_level("WARNING", logger="pm.plugin_eviction"): - cfg, _ = _evict(tmp_path, ["web/firecrawl"], "plugins:\n enabled: [web/firecrawl, firecrawl, keep]\n") - assert cfg["plugins"]["enabled"] == ["keep"] - assert "tree unreadable" in caplog.text - - -def test_unexpected_alias_discovery_error_is_not_swallowed(tmp_path, monkeypatch): - import hermes_cli.plugins_cmd as plugins_cmd - - def broken(): - raise RuntimeError("half-broken plugin tree") - - monkeypatch.setattr(plugins_cmd, "_discover_all_plugins", broken) - with pytest.raises(RuntimeError, match="half-broken plugin tree"): - _evict(tmp_path, ["plug-a"], "plugins:\n enabled: [plug-a]\n") - - -def test_real_alias_discovery_skips_a_malformed_manifest_and_still_removes_aliases(tmp_path, monkeypatch): - monkeypatch.setenv("HERMES_HOME", str(tmp_path)) - monkeypatch.setattr(_plugins_cmd, "_plugin_aliases", _REAL_ALIASES) - monkeypatch.setattr(_plugins_cmd, "_discover_all_plugins", _REAL_DISCOVER) - good = tmp_path / "plugins" / "web" / "firecrawl" - good.mkdir(parents=True) - (good / "plugin.yaml").write_text("name: web-firecrawl\n", encoding="utf-8") - bad = tmp_path / "plugins" / "broken" - bad.mkdir(parents=True) - (bad / "plugin.yaml").write_text("name: [unterminated\n", encoding="utf-8") - cfg, _ = _evict(tmp_path, ["web/firecrawl"], - "plugins:\n enabled: [web/firecrawl, web-firecrawl, firecrawl, keep]\n") - assert cfg["plugins"]["enabled"] == ["keep"] - - -def test_several_evicted_plugins_share_one_plugin_tree_scan(tmp_path, monkeypatch): - import hermes_cli.plugins_cmd as plugins_cmd - - scans = [] - monkeypatch.setattr(plugins_cmd, "_discover_all_plugins", lambda: scans.append(1) or []) - cfg, _ = _evict(tmp_path, ["plug-a", "plug-b", "plug-c"], - "plugins:\n enabled: [plug-a, plug-b, plug-c, keep]\n") - assert cfg["plugins"]["enabled"] == ["keep"] - assert len(scans) == 1 - - -def test_no_tree_scan_when_nothing_is_enabled(tmp_path, monkeypatch): - import hermes_cli.plugins_cmd as plugins_cmd - - scans = [] - monkeypatch.setattr(plugins_cmd, "_discover_all_plugins", lambda: scans.append(1) or []) - _evict(tmp_path, ["plug-a"], "plugins:\n disabled: []\n") - assert scans == [] diff --git a/tests/pm/test_plugin_survival_contract.py b/tests/pm/test_plugin_survival_contract.py index 292d4b10ae..22b2f699b9 100644 --- a/tests/pm/test_plugin_survival_contract.py +++ b/tests/pm/test_plugin_survival_contract.py @@ -370,7 +370,8 @@ def test_plugin_whose_pyproject_installs_nothing_keeps_the_environment(admission def test_update_sync_disables_plugin_excluded_by_requires_python(admission_env): """An update never fails because of a plugin: a member whose requires-python excludes the interpreter core now runs on is disabled in every home that enables it (plugins.enabled in - one, memory.provider in another), the rest build, and the next boot sees a current venv.""" + one, memory.provider in another), the rest build, and the next boot sees a current venv. + The provider selection survives, and the update after the plugin fits again undoes the edit.""" from pm.environments import runtime_facts_path, selected_venv from pm.install import sync_venv, venv_is_current from pm.lock import Facts @@ -400,11 +401,51 @@ def test_update_sync_disables_plugin_excluded_by_requires_python(admission_env): assert "# operator note" in text assert "too-old" in yaml.safe_load(text)["plugins"]["disabled"] assert yaml.safe_load(text)["plugins"]["enabled"] == ["fits"] - assert yaml.safe_load((profile / "config.yaml").read_text(encoding="utf-8"))["memory"]["provider"] == "" + work = yaml.safe_load((profile / "config.yaml").read_text(encoding="utf-8")) + assert work["memory"]["provider"] == "too-old" and work["plugins"]["disabled"] == ["too-old"] assert "too-old" in json.dumps(_latest_receipt(home).get("warnings")) assert "hermes plugins enable too-old" in json.dumps(_latest_receipt(home).get("warnings")) assert venv_is_current(project_root=core) is True + for plugin in (home / "plugins" / "too-old", profile / "plugins" / "too-old"): + manifest = plugin / "pyproject.toml" + manifest.write_text(manifest.read_text(encoding="utf-8").replace( + f'"<{sys.version_info[0]}.{sys.version_info[1]}"', '">=3.11"'), encoding="utf-8") + sync_venv(explicit=True, evict_incompatible_plugins=True) + + root = yaml.safe_load((home / "config.yaml").read_text(encoding="utf-8"))["plugins"] + assert root["enabled"] == ["fits", "too-old"] and not root.get("disabled") and "disabled_by_update" not in root + work = yaml.safe_load((profile / "config.yaml").read_text(encoding="utf-8")) + assert work["memory"]["provider"] == "too-old" and not work["plugins"].get("enabled") + assert not work["plugins"].get("disabled") and "disabled_by_update" not in work["plugins"] + workspace = Path(Facts(runtime_facts_path(core), strict=True).get("venv")["resolved_lock"]).parent + assert "too-old" in (workspace / "pyproject.toml").read_text() + assert venv_is_current(project_root=core) is True + + +@pytest.mark.skipif(not _uv_available(), reason="uv not on PATH") +def test_plugin_the_user_disables_after_an_update_stays_disabled(admission_env): + """Disabling a plugin an update turned off takes the decision back: later updates stop + retrying it, even once it fits again.""" + from pm.install import sync_venv + from pm.plugin_inputs import Selection + + _, home = admission_env + _local_conflict_members(home) + _write_enabled(home, ["plug-a", "plug-b"]) + sync_venv(explicit=True, evict_incompatible_plugins=True) + assert "plug-b" in yaml.safe_load((home / "config.yaml").read_text(encoding="utf-8"))["plugins"]["disabled_by_update"] + + sync_venv(explicit=True, plugins=Selection({"home": str(home.resolve()), "enabled": ["plug-a"], + "disabled": ["plug-b"], "settled": ["plug-b"]})) + cfg = yaml.safe_load((home / "config.yaml").read_text(encoding="utf-8")) + cfg["plugins"]["enabled"] = [] # without plug-a, plug-b would fit again + (home / "config.yaml").write_text(yaml.safe_dump(cfg), encoding="utf-8") + sync_venv(explicit=True, evict_incompatible_plugins=True) + + plugins = yaml.safe_load((home / "config.yaml").read_text(encoding="utf-8"))["plugins"] + assert plugins["disabled"] == ["plug-b"] and "disabled_by_update" not in plugins + @pytest.mark.skipif(not _uv_available(), reason="uv not on PATH") def test_update_sync_disables_later_plugin_of_unresolvable_union(admission_env):