From aeff56aa3583fd6500a9d5f75ce1421dc93b679f Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Mon, 21 Sep 2026 22:38:28 -0700 Subject: [PATCH] fix(plugins): loader gates read the running version, survive SystemExit, quarantine deps everywhere - requires_hermes compared against stale editable-install dist metadata (0.21.0) while the checkout ran 0.21.4, skipping plugins that required the release in use; hermes_cli.__version__ is now the source of truth, dist metadata only a fallback. - PEP 440 pre/post suffixes glued to a segment (99.0.0rc1) made a clause permissive and an rc running version disabled every gate; the segment parser drops the suffix. - A plugin calling sys.exit() at import or in register() propagated SystemExit out of discovery: the whole registry emptied and `hermes chat` exited 3 with no output. Load isolation now covers SystemExit (KeyboardInterrupt still propagates). - uv reads [tool.uv] exclude-newer from the cwd project only, so lazy/plugin dep installs launched from $HOME, a gateway service or the Desktop backend were never quarantined; the uv tier now runs from the checkout root when one exists. - A flat user/project manifest naming a bundled key from a differently named directory no longer displaces the bundled plugin (warn + skip); a same-named override is logged at INFO. --- hermes_cli/plugins.py | 7 ++++--- hermes_cli/plugins_discovery.py | 28 +++++++++++++++++++++++++++- hermes_cli/plugins_loader.py | 23 ++++++++++++++++------- hermes_cli/plugins_manifest.py | 31 ++++++++++++++++++++++--------- tools/lazy_deps.py | 12 +++++++++++- 5 files changed, 80 insertions(+), 21 deletions(-) diff --git a/hermes_cli/plugins.py b/hermes_cli/plugins.py index 15fa39d715..8d8bfddf5c 100644 --- a/hermes_cli/plugins.py +++ b/hermes_cli/plugins.py @@ -43,7 +43,7 @@ from hermes_cli.plugins_manifest import ( # noqa: F401 — re-exported ) from hermes_cli.plugins_discovery import ( # noqa: F401 — re-exported ENTRY_POINTS_GROUP, _get_disabled_plugins, _get_enabled_plugins, collect_directory_manifests, - discover_entrypoint_manifests, gate_manifest, scan_directory, + discover_entrypoint_manifests, gate_manifest, resolve_manifest_winners, scan_directory, ) from hermes_cli.plugins_loader import ( PluginLoaderMixin, _BARE_MODULE_SCOPE, _MODULE_NAMESPACE_LOCK, _NS_PARENT, _evict_modules, @@ -1332,9 +1332,10 @@ class PluginManager(PluginLoaderMixin, PluginDispatchMixin, PluginLedgerMixin): logger.warning("Removed Hermes plugin %s is still listed in plugins.enabled; " "remove it and configure native Relay plugins with %s", ", ".join(stale_relay_keys), RELAY_PLUGINS_CONFIG_ENV) - # Later sources win on key collision (project > user > bundled); gate the winners, then + # Later sources win on key collision (project > user > bundled) except a flat impostor claiming a + # bundled key from another directory (resolve_manifest_winners); gate the winners, then # load survivors in requires_plugins order (see resolve_plugin_load_order). - winners = {manifest_key(m): m for m in manifests} + winners = resolve_manifest_winners(manifests) to_load = {k: m for k, m in winners.items() if self._gate_manifest(m, disabled, enabled)} for lookup_key in resolve_plugin_load_order(to_load): manifest = to_load[lookup_key] diff --git a/hermes_cli/plugins_discovery.py b/hermes_cli/plugins_discovery.py index dd38a5edb8..de17dec580 100644 --- a/hermes_cli/plugins_discovery.py +++ b/hermes_cli/plugins_discovery.py @@ -10,7 +10,7 @@ import importlib.metadata import logging from dataclasses import dataclass from pathlib import Path -from typing import Any, List, Optional, Set +from typing import Any, Dict, List, Optional, Set from hermes_constants import get_hermes_home from hermes_cli.config import cfg_get @@ -167,6 +167,32 @@ def collect_directory_manifests() -> List[PluginManifest]: return manifests +def resolve_manifest_winners(manifests: List[PluginManifest]) -> Dict[str, PluginManifest]: + """Later sources win on key collision (project > user > bundled): a same-named copy under + ``~/.hermes/plugins/`` is the documented way to override a bundled plugin, and is logged. A flat + user/project manifest that claims a bundled key from a *differently named* directory is an impostor, not + an override (``impostor_dir/plugin.yaml`` with ``name: kanban``): it is skipped with a warning so + ``hermes plugins enable kanban`` never activates unrelated code under the bundled name.""" + winners: Dict[str, PluginManifest] = {} + for manifest in manifests: + key = manifest_key(manifest) + shadowed = winners.get(key) + if shadowed is not None and shadowed.source == "bundled" and manifest.source in {"user", "project"}: + own_dir = Path(manifest.path).name if manifest.path else "" + bundled_dir = Path(shadowed.path).name if shadowed.path else "" + if own_dir and bundled_dir and own_dir != bundled_dir: + logger.warning( + "Ignoring %s plugin at %s: its manifest name '%s' is a bundled plugin's key but the " + "directory is named '%s'; rename the directory to '%s' to override the bundled plugin", + manifest.source, manifest.path, key, own_dir, bundled_dir, + ) + continue + logger.info("Plugin '%s' at %s (%s) shadows the bundled copy at %s", key, manifest.path, + manifest.source, shadowed.path) + winners[key] = manifest + return winners + + @dataclass(frozen=True) class ManifestGate: """Routing verdict for one winning manifest (see :func:`gate_manifest`).""" diff --git a/hermes_cli/plugins_loader.py b/hermes_cli/plugins_loader.py index 3af0120ebf..416eb12f1a 100644 --- a/hermes_cli/plugins_loader.py +++ b/hermes_cli/plugins_loader.py @@ -64,6 +64,13 @@ def _plugin_home_scope(home: Path): reset_hermes_home_override(token) +def _load_error_text(exc: BaseException) -> str: + """Human-readable load failure; ``sys.exit(0)`` has an empty ``str()`` so name the class and code.""" + if isinstance(exc, SystemExit): + return f"SystemExit({exc.code!r}) raised during import/register()" + return str(exc) + + def _dist_installed(req: str) -> Optional[bool]: """Best-effort presence probe on a requirement's distribution name; ``None`` when unprobeable.""" dist = re.split(r"[<>=!~\[;\s]", req, maxsplit=1)[0].strip() @@ -199,7 +206,7 @@ class PluginLoaderMixin: "Deferred platform '%s': pre-registered %d client tool(s) %s", lookup_key, len(registered), registered, ) - except Exception as exc: + except (Exception, SystemExit) as exc: # Tools registered before the raise are live: credit them or `hermes plugins list` under-reports # (and _load_plugin's later diff would miss them too). Never break discovery (the platform stays # deferred), but a broken tools.py IS the symptom, so warn — and say where it failed first. @@ -314,15 +321,17 @@ class PluginLoaderMixin: from hermes_cli.plugins_ledger import _hook_source_of self._drop_fallback_hooks(_hook_source_of(manifest.name, module)) - except Exception as exc: + except (Exception, SystemExit) as exc: + # SystemExit too: a plugin module with an unguarded ``main()``/``sys.exit()`` must not take the + # whole process (and every other plugin's registry) down with it; KeyboardInterrupt still propagates. owned = [r for r in self._registration_order if r.plugin_key == plugin_key] self._dispose_registrations(owned) self._forget_registrations(owned) - loaded.error = str(exc) + loaded.error = _load_error_text(exc) # register() may have subscribed before raising; a failed plugin must leave no callable reachable # from later event dispatch. self._remove_plugin_subscriptions(plugin_key) - logger.warning("Failed to load plugin '%s': %s", manifest.name, exc, exc_info=_PLUGINS_DEBUG) + logger.warning("Failed to load plugin '%s': %s", manifest.name, _load_error_text(exc), exc_info=_PLUGINS_DEBUG) # The failure path swept this plugin's whole ledger (not just the registration_start slice), so # discovery-time pre-registrations are gone too. # There is no live tool left to credit — attribution and the registry agree at zero. Only the @@ -400,9 +409,9 @@ class PluginLoaderMixin: continue self._portable_mcp_servers[internal_name] = dict(config) loaded.enabled = True - except Exception as exc: - loaded.error = str(exc) - logger.warning("Failed to load Agent Plugin '%s': %s", lookup_key, exc) + except (Exception, SystemExit) as exc: + loaded.error = _load_error_text(exc) + logger.warning("Failed to load Agent Plugin '%s': %s", lookup_key, loaded.error) self._plugins[lookup_key] = loaded def _directory_module_name(self, manifest: PluginManifest) -> str: diff --git a/hermes_cli/plugins_manifest.py b/hermes_cli/plugins_manifest.py index 92b36904b3..1fd45dc037 100644 --- a/hermes_cli/plugins_manifest.py +++ b/hermes_cli/plugins_manifest.py @@ -375,22 +375,35 @@ _VERSION_COMPARATOR_RE = re.compile(r"^\s*(>=|<=|==|!=|>|<)\s*(.+?)\s*$") def running_hermes_version() -> str: - """Installed ``hermes-agent`` distribution version, else ``hermes_cli.__version__`` (source checkout).""" + """Version of the Hermes code that is running: ``hermes_cli.__version__``. Distribution metadata is only + a fallback — on an editable/source install it is frozen at ``pip install -e`` time and drifts from the + checkout after every ``git pull`` (dist said 0.21.0 while the code was 0.21.4), so gating on it skipped + plugins that required exactly the release the user was running.""" try: - return importlib.metadata.version("hermes-agent") - except Exception: from hermes_cli import __version__ - return __version__ + if __version__: + return str(__version__) + except Exception: + pass + return importlib.metadata.version("hermes-agent") + + +_VERSION_SEGMENT_RE = re.compile(r"^\d+") def _version_tuple(v: str) -> Optional[tuple]: - """``v1.2.3-rc1`` → ``(1, 2, 3)``; ``None`` when a segment is non-numeric.""" + """``v1.2.3-rc1`` / ``1.2.3rc1`` / ``1.2.3.post1`` → ``(1, 2, 3)``; ``None`` when a segment has no + leading digits. PEP 440 pre/post/dev suffixes glued to a segment (``0rc1``) are dropped so an rc + *target* still gates and an rc *running* version does not disable every gate.""" parts = re.split(r"[-+]", str(v).strip().lstrip("v"), 1)[0].split(".") parts += ["0"] * (3 - len(parts)) - try: - return tuple(int(x) for x in parts[:3]) - except ValueError: - return None + out = [] + for x in parts[:3]: + m = _VERSION_SEGMENT_RE.match(x.strip()) + if m is None: + return None + out.append(int(m.group(0))) + return tuple(out) def version_satisfies(spec: str, current: str) -> bool: diff --git a/tools/lazy_deps.py b/tools/lazy_deps.py index 675dd5d21f..c9b4e16c02 100644 --- a/tools/lazy_deps.py +++ b/tools/lazy_deps.py @@ -528,6 +528,15 @@ def _run_installer(cmd: list[str], **kw) -> subprocess.CompletedProcess: return subprocess.run(cmd, **_SUBPROCESS_KW, creationflags=windows_hide_flags(), **kw) +def _uv_policy_cwd() -> Optional[str]: + """Directory uv must run from so the checkout's ``[tool.uv]`` policy (``exclude-newer`` quarantine and its + per-package exceptions) applies: uv reads it from the *current directory's* project only, so a lazy or + plugin install launched from ``$HOME``, a gateway service or the Desktop backend was never quarantined. + ``None`` (inherit cwd) when this is not a source checkout.""" + root = Path(__file__).resolve().parent.parent + return str(root) if (root / "pyproject.toml").is_file() else None + + def _uv_binary() -> Optional[str]: """Managed uv first ($HERMES_HOME/bin is never on PATH), then PATH. A lookup, not ensure_uv(): downloading uv mid-turn is more than the caller asked for; pip covers no-uv.""" @@ -599,7 +608,8 @@ def _venv_pip_install(specs: tuple[str, ...], *, timeout: int = 300, constraint_ if pip_index_url: uv_env["UV_INDEX_URL"] = pip_index_url try: - r = _run_installer([uv_bin, "pip", "install", "--compile-bytecode", *extra_args, *specs], timeout=timeout, env=uv_env) + r = _run_installer([uv_bin, "pip", "install", "--compile-bytecode", *extra_args, *specs], + timeout=timeout, env=uv_env, cwd=_uv_policy_cwd()) if r.returncode != 0: logger.debug("uv pip install failed: %s", r.stderr) # A uv resolver failure is authoritative: falling through to pip would discard uv