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.
This commit is contained in:
@@ -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]
|
||||
|
||||
@@ -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/<name>`` 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`)."""
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user