diff --git a/hermes_cli/plugin_dev.py b/hermes_cli/plugin_dev.py index 6484caa0b9..ae470285b0 100644 --- a/hermes_cli/plugin_dev.py +++ b/hermes_cli/plugin_dev.py @@ -218,6 +218,79 @@ def _accepts_var_kwargs(callback: Any) -> bool: return any(parameter.kind is inspect.Parameter.VAR_KEYWORD for parameter in parameters) +def _check_manifest_v2(report: "DoctorReport", manifest: Any) -> None: + """Manifest v2 (#64165) checks: versions, deps, pip declarations, schema.""" + import importlib.metadata + import re as _re + + from hermes_cli.plugins import SUPPORTED_MANIFEST_VERSION + + mv = getattr(manifest, "manifest_version", 1) + if mv > SUPPORTED_MANIFEST_VERSION: + report.warning( + f"manifest_version {mv} is newer than this Hermes supports " + f"({SUPPORTED_MANIFEST_VERSION}); unknown fields are ignored" + ) + + api_version = getattr(manifest, "api_version", None) + if api_version is not None and api_version < 1: + report.warning(f"api_version {api_version} is not a valid API generation (>= 1)") + + for dep in getattr(manifest, "requires_plugins", []) or []: + dep_id = dep.get("id") if isinstance(dep, dict) else None + if not dep_id: + report.warning(f"requires_plugins entry {dep!r} has no plugin id") + continue + vr = dep.get("version_range") + if vr: + report.warning( + f"requires plugin {dep_id!r} ({vr}) — version ranges are " + "advisory; a missing dependency logs a warning at load" + ) + + pydeps = getattr(manifest, "python_dependencies", []) or [] + missing: list[str] = [] + unpinned: list[str] = [] + for req in pydeps: + dist = _re.split(r"[<>=!~\[;\s]", req, maxsplit=1)[0].strip() + if not _re.search(r"<|==|~=", req): + unpinned.append(req) + if not dist: + continue + try: + importlib.metadata.version(dist) + except importlib.metadata.PackageNotFoundError: + missing.append(req) + except Exception: + continue + for req in unpinned: + report.warning( + f"python_dependencies entry {req!r} has no upper bound — " + "pin an upper bound (e.g. 'pkg>=1.0,<2') per the dependency policy" + ) + if missing: + report.warning( + "declared python_dependencies not installed: " + + ", ".join(missing) + + " — Hermes never auto-installs plugin dependencies; " + + "install manually: pip install " + + " ".join(f"'{m}'" for m in missing) + ) + + schema = getattr(manifest, "config_schema", {}) or {} + if schema: + from hermes_cli.plugins import _CONFIG_SCHEMA_TYPES + + for skey, spec in schema.items(): + if not isinstance(spec, dict): + continue + stype = spec.get("type") + if stype is not None and str(stype).lower() not in _CONFIG_SCHEMA_TYPES: + report.warning( + f"config_schema key {skey!r} declares unknown type {stype!r}" + ) + + def doctor_plugin(target: str | os.PathLike[str] | None = None) -> DoctorReport: """Validate one plugin through Hermes' real scanner and registration path.""" try: @@ -275,6 +348,8 @@ def doctor_plugin(target: str | os.PathLike[str] | None = None) -> DoctorReport: report.warning(f"manifest declares tool {name!r} but registration did not add it") for name in sorted(registered_tool_names - declared_tool_names): report.warning(f"registration adds tool {name!r} not listed in provides_tools") + + _check_manifest_v2(report, host.manifest) except _DoctorLoadError as exc: report.error(str(exc)) except Exception as exc: diff --git a/hermes_cli/plugins.py b/hermes_cli/plugins.py index b281e891f2..94d13fac18 100644 --- a/hermes_cli/plugins.py +++ b/hermes_cli/plugins.py @@ -368,6 +368,282 @@ def _display_author(value: object) -> str: return "" if value is None else str(value) +# ── Manifest v2 (#64165) parsing helpers ────────────────────────────────── + +# Fields the current parser understands. Anything else in plugin.yaml is +# forward-compat surface: warn (once per manifest, at debug for v1 files to +# avoid churning existing plugins, at warning for v2+) and continue loading. +_KNOWN_MANIFEST_FIELDS: Set[str] = { + # v1 + "name", "version", "description", "author", "requires_env", + "provides_tools", "provides_hooks", "kind", "hooks", "label", + "optional_env", "platforms", "external_dependencies", "pip_dependencies", + "provides_browser_providers", "provides_web_providers", + # v2 (#64165) + "manifest_version", "api_version", "requires_plugins", + "python_dependencies", "config_schema", "license", "homepage", "tags", + # owned by sibling sub-issues but reserved so their manifests don't warn + "capabilities", "emits", "listens", "hermes", "depends", +} + +# Highest manifest schema version this Hermes understands. +SUPPORTED_MANIFEST_VERSION = 2 + +_CONFIG_SCHEMA_TYPES: Dict[str, tuple] = { + "str": (str,), + "string": (str,), + "int": (int,), + "integer": (int,), + "float": (int, float), + "number": (int, float), + "bool": (bool,), + "boolean": (bool,), + "list": (list,), + "array": (list,), + "dict": (dict,), + "object": (dict,), +} + + +def _parse_manifest_v2_fields(data: Mapping, key: str) -> Dict[str, Any]: + """Validate and normalize the manifest v2 fields (#64165). + + Returns kwargs for :class:`PluginManifest`. Every problem is a warning, + never a load failure — v2 metadata is advisory and additive. + """ + out: Dict[str, Any] = {} + + # manifest_version — absent means v1 (supported forever). + raw_mv = data.get("manifest_version", 1) + try: + mv = int(raw_mv) + except (TypeError, ValueError): + logger.warning( + "Plugin %s: manifest_version %r is not an integer; treating as 1", + key, raw_mv, + ) + mv = 1 + if mv > SUPPORTED_MANIFEST_VERSION: + logger.warning( + "Plugin %s: manifest_version %d is newer than this Hermes " + "supports (%d); loading anyway and ignoring unknown fields", + key, mv, SUPPORTED_MANIFEST_VERSION, + ) + out["manifest_version"] = mv + + # api_version — plugin API generation (independent of manifest_version). + raw_api = data.get("api_version") + if raw_api is None: + out["api_version"] = None + else: + try: + out["api_version"] = int(raw_api) + except (TypeError, ValueError): + logger.warning( + "Plugin %s: api_version %r is not an integer; ignoring", key, raw_api, + ) + out["api_version"] = None + + # requires_plugins — list of {id, version_range?} (str shorthand ok). + deps: List[Dict[str, Any]] = [] + raw_deps = data.get("requires_plugins") + if raw_deps is not None and not isinstance(raw_deps, list): + logger.warning( + "Plugin %s: requires_plugins must be a list; ignoring", key, + ) + raw_deps = None + for item in raw_deps or []: + if isinstance(item, str): + deps.append({"id": item, "version_range": None}) + elif isinstance(item, Mapping) and isinstance(item.get("id"), str) and item["id"]: + vr = item.get("version_range") + deps.append({ + "id": item["id"], + "version_range": str(vr) if vr is not None else None, + }) + else: + logger.warning( + "Plugin %s: requires_plugins entry %r must be a plugin id " + "string or a {id, version_range} mapping; skipping", key, item, + ) + out["requires_plugins"] = deps + + # python_dependencies — declared pip requirement strings. Validated and + # surfaced ONLY; never auto-installed (isolation design deferred). + pydeps: List[str] = [] + raw_pydeps = data.get("python_dependencies") + if raw_pydeps is not None and not isinstance(raw_pydeps, list): + logger.warning( + "Plugin %s: python_dependencies must be a list of requirement " + "strings; ignoring", key, + ) + raw_pydeps = None + for item in raw_pydeps or []: + if isinstance(item, str) and item.strip(): + pydeps.append(item.strip()) + else: + logger.warning( + "Plugin %s: python_dependencies entry %r must be a non-empty " + "requirement string; skipping", key, item, + ) + out["python_dependencies"] = pydeps + + # config_schema — mapping of key -> {type?, default?, description?, required?}. + raw_schema = data.get("config_schema") + schema: Dict[str, Any] = {} + if raw_schema is not None and not isinstance(raw_schema, Mapping): + logger.warning( + "Plugin %s: config_schema must be a mapping; ignoring", key, + ) + raw_schema = None + for skey, spec in (raw_schema or {}).items(): + if not isinstance(spec, Mapping): + logger.warning( + "Plugin %s: config_schema entry %r must be a mapping " + "(e.g. {type: str}); skipping", key, skey, + ) + continue + stype = spec.get("type") + if stype is not None and str(stype).lower() not in _CONFIG_SCHEMA_TYPES: + logger.warning( + "Plugin %s: config_schema key %r declares unknown type %r " + "(known: %s); type check will be skipped for it", + key, skey, stype, ", ".join(sorted(_CONFIG_SCHEMA_TYPES)), + ) + schema[str(skey)] = dict(spec) + out["config_schema"] = schema + + # Standard metadata. + out["license"] = str(data.get("license") or "") + out["homepage"] = str(data.get("homepage") or "") + raw_tags = data.get("tags") + if raw_tags is not None and not isinstance(raw_tags, list): + logger.warning("Plugin %s: tags must be a list; ignoring", key) + raw_tags = None + out["tags"] = [str(t) for t in (raw_tags or [])] + + # Forward compat: unknown fields warn (never fail). Keep v1 manifests + # quiet at warning level — they predate the known-field census. + unknown = sorted(set(data.keys()) - _KNOWN_MANIFEST_FIELDS) + if unknown: + log = logger.warning if mv >= 2 else logger.debug + log( + "Plugin %s: unknown manifest field(s) ignored: %s " + "(newer manifest schema or typo; plugin still loads)", + key, ", ".join(unknown), + ) + + return out + + +def validate_config_schema( + plugin_id: str, + schema: Mapping, + settings: Mapping, +) -> List[str]: + """Validate a plugin's config entry against its declared config_schema. + + Returns a list of human-actionable warning strings. Never raises; + schema mismatches must not block plugin load (#64165). + """ + warnings: List[str] = [] + if not isinstance(schema, Mapping) or not isinstance(settings, Mapping): + return warnings + for skey, spec in schema.items(): + if not isinstance(spec, Mapping): + continue + present = skey in settings + if not present: + if spec.get("required") and "default" not in spec: + warnings.append( + f"plugins.entries.{plugin_id}.settings.{skey} is required " + "by the plugin's config_schema but is not set" + ) + continue + stype = spec.get("type") + expected = _CONFIG_SCHEMA_TYPES.get(str(stype).lower()) if stype else None + if expected is not None: + value = settings[skey] + # bool is an int subclass — don't let True satisfy int/float. + ok = isinstance(value, expected) and not ( + isinstance(value, bool) and bool not in expected + ) + if not ok: + warnings.append( + f"plugins.entries.{plugin_id}.settings.{skey} should be " + f"{stype} (got {type(value).__name__})" + ) + return warnings + + +def resolve_plugin_load_order( + manifests: Mapping[str, "PluginManifest"], +) -> List[str]: + """Return plugin keys in dependency-respecting load order (#64165). + + When A requires B, B sorts before A (so B's ``register()`` runs first). + Ties break alphabetically for determinism. Dependency cycles are + detected, warned about, and the members of the cycle fall back to + alphabetical order after every non-cycle plugin they depend on. + Missing dependencies are warned about here (once, at discovery) but do + not remove the dependent plugin from the order — loads never hard-fail + on a missing advisory dependency. + """ + import graphlib + + keys = sorted(manifests.keys()) + by_name: Dict[str, str] = {} + for k in keys: + name = manifests[k].name + if name and name not in by_name: + by_name[name] = k + + def _resolve_dep(dep_id: str) -> Optional[str]: + if dep_id in manifests: + return dep_id + return by_name.get(dep_id) + + edges: Dict[str, Set[str]] = {k: set() for k in keys} + for k in keys: + for dep in manifests[k].requires_plugins: + dep_id = dep.get("id") if isinstance(dep, Mapping) else None + if not dep_id: + continue + resolved = _resolve_dep(dep_id) + if resolved is None: + logger.warning( + "Plugin %s requires plugin '%s' which is not enabled/" + "installed; loading anyway (probe availability at runtime " + "via ctx.has_plugin). Run `hermes plugins enable %s` if " + "it is installed.", + k, dep_id, dep_id, + ) + continue + if resolved == k: + logger.warning("Plugin %s declares a dependency on itself; ignoring", k) + continue + edges[k].add(resolved) + + sorter = graphlib.TopologicalSorter(edges) + try: + sorter.prepare() + except graphlib.CycleError as exc: + cycle = exc.args[1] if len(exc.args) > 1 else [] + logger.warning( + "Plugin dependency cycle detected (%s); falling back to " + "alphabetical load order for all plugins", + " -> ".join(str(c) for c in cycle), + ) + return keys + + ordered: List[str] = [] + while sorter.is_active(): + ready = sorted(sorter.get_ready()) + ordered.extend(ready) + sorter.done(*ready) + return ordered + + @dataclass class PluginManifest: """Parsed representation of a plugin.yaml manifest.""" @@ -412,6 +688,31 @@ class PluginManifest: # granted it (``plugins.entries..granted_capabilities``) or the # deprecated legacy ``allow_*`` key is set. capabilities: List[str] = field(default_factory=list) + # ── Manifest v2 fields (#64165) — all optional and additive ────────── + # Manifest SCHEMA version. Absent (v1) manifests are fully supported + # forever. This versions the *file format* only; it is deliberately + # independent from ``api_version`` (the runtime plugin API generation). + manifest_version: int = 1 + # Runtime plugin API generation the plugin targets (ctx surface / + # hook signatures). ``None`` = unspecified (treated as current-compatible). + api_version: Optional[int] = None + # Inter-plugin dependencies: list of {"id": str, "version_range": str|None}. + # Advisory: a missing dependency logs a warning but the plugin still + # loads (plugins can probe availability via ``ctx.has_plugin``). Load + # ORDER honors these edges: if A requires B, B registers first. + requires_plugins: List[Dict[str, Any]] = field(default_factory=list) + # Declared pip dependencies. VALIDATED AND SURFACED ONLY — Hermes never + # auto-installs these (isolation design for the install seam is a + # deferred follow-up; see #64165 round-2 review and #15220). + python_dependencies: List[str] = field(default_factory=list) + # JSON-schema-ish mapping describing keys under + # ``plugins.entries..settings``. Validated at load; mismatches are + # warnings, never load failures. + config_schema: Dict[str, Any] = field(default_factory=dict) + # Formalized standard metadata. + license: str = "" + homepage: str = "" + tags: List[str] = field(default_factory=list) @dataclass(frozen=True) @@ -654,6 +955,20 @@ class PluginContext: """Return the effective registry id used for this plugin's namespaces.""" return self.manifest.key or self.manifest.name + def has_plugin(self, plugin_id: str) -> bool: + """Return True when another plugin is loaded and enabled (#64165). + + Companion to the advisory ``requires_plugins`` manifest field: a + missing dependency never blocks load, so plugins probe availability + at runtime with this. Matches on registry key or manifest name. + """ + for key, loaded in self._manager._plugins.items(): + if not loaded.enabled: + continue + if key == plugin_id or loaded.manifest.name == plugin_id: + return True + return False + # -- namespaced config and durable state -------------------------------- def get_config(self, key: str, default: Any = None) -> Any: @@ -2019,6 +2334,10 @@ class PluginManager: winners: Dict[str, PluginManifest] = {} for manifest in manifests: winners[manifest.key or manifest.name] = manifest + # Standalone/user plugins that pass the gates below are collected + # here and loaded AFTER the sweep in dependency-respecting order + # (requires_plugins topological sort, #64165). + to_load: Dict[str, PluginManifest] = {} for manifest in winners.values(): lookup_key = manifest.key or manifest.name @@ -2103,6 +2422,16 @@ class PluginManager: "Skipping '%s' (not in plugins.enabled)", lookup_key ) continue + to_load[lookup_key] = manifest + + # Load the surviving standalone plugins in dependency order: + # when A requires B, B's register() runs before A's (topological + # sort, stable alphabetical tiebreak; cycles warn and fall back to + # alphabetical order). Missing deps warn but never block the load. + for lookup_key in resolve_plugin_load_order(to_load): + manifest = to_load[lookup_key] + self._warn_python_dependencies(manifest) + self._validate_plugin_config_schema(manifest) self._load_plugin(manifest) if manifests: @@ -2449,6 +2778,7 @@ class PluginManager: "Parsed manifest: key=%s name=%s kind=%s source=%s path=%s", key, name, kind, source, plugin_dir, ) + v2_fields = _parse_manifest_v2_fields(data, key) return PluginManifest( name=name, version=str(data.get("version", "")), @@ -2464,6 +2794,7 @@ class PluginManager: capabilities=_parse_declared_capabilities( data.get("capabilities"), name ), + **v2_fields, ) except Exception as exc: logger.warning( @@ -2565,6 +2896,73 @@ class PluginManager: ) self._load_plugin(manifest) + def _warn_python_dependencies(self, manifest: PluginManifest) -> None: + """Surface declared pip dependencies (#64165). + + python_dependencies is a declaration seam ONLY: Hermes validates and + prints the requirements with an install hint but NEVER auto-installs + them. The isolation design (constraints installs vs. vendored dirs + vs. conflict-detection-and-refusal) is an explicitly deferred + follow-up — see the round-2 review on #64165 and #15220. + """ + deps = manifest.python_dependencies + if not deps: + return + key = manifest.key or manifest.name + missing: List[str] = [] + for req in deps: + # Best-effort presence probe on the distribution name. + dist = re.split(r"[<>=!~\[;\s]", req, maxsplit=1)[0].strip() + if not dist: + continue + try: + importlib.metadata.version(dist) + except importlib.metadata.PackageNotFoundError: + missing.append(req) + except Exception: + continue + if missing: + logger.warning( + "Plugin %s declares Python dependencies that are not " + "installed: %s. Hermes does not install plugin dependencies " + "automatically; install them yourself, e.g.: pip install %s", + key, ", ".join(missing), + " ".join(f"'{m}'" for m in missing), + ) + else: + logger.debug( + "Plugin %s python_dependencies satisfied: %s", + key, ", ".join(deps), + ) + + def _validate_plugin_config_schema(self, manifest: PluginManifest) -> None: + """Check plugins.entries. settings against config_schema (#64165). + + Mismatches log actionable warnings naming the key and expected type; + they never block the plugin from loading. + """ + if not manifest.config_schema: + return + plugin_id = manifest.key or manifest.name + settings: Mapping[str, Any] = {} + try: + from hermes_cli.config import load_config + + cfg = load_config() or {} + entries = (cfg.get("plugins") or {}).get("entries") or {} + entry = entries.get(plugin_id) if isinstance(entries, Mapping) else None + raw = entry.get("settings") if isinstance(entry, Mapping) else None + if not isinstance(raw, Mapping): + # Migration fallback mirroring ctx.get_config. + raw = entry.get("config") if isinstance(entry, Mapping) else None + settings = raw if isinstance(raw, Mapping) else {} + except Exception: + settings = {} + for warning in validate_config_schema( + plugin_id, manifest.config_schema, settings + ): + logger.warning("Plugin %s config: %s", plugin_id, warning) + def _load_plugin(self, manifest: PluginManifest) -> None: """Import a plugin module and call its ``register(ctx)`` function.""" loaded = LoadedPlugin(manifest=manifest) diff --git a/hermes_cli/plugins_cmd.py b/hermes_cli/plugins_cmd.py index 7ddd23a098..56f54e683b 100644 --- a/hermes_cli/plugins_cmd.py +++ b/hermes_cli/plugins_cmd.py @@ -331,6 +331,32 @@ def _missing_requires_env_names(manifest: dict) -> list[str]: return [s["name"] for s in env_specs if s.get("name") and not get_env_value(s["name"])] +def _print_python_dependencies(manifest: dict, console) -> None: + """Surface declared python_dependencies at install time (#64165). + + Declaration seam ONLY — Hermes never auto-installs plugin pip + dependencies (isolation design deferred; see #64165 / #15220). We print + the declared requirements with a copy-pasteable install hint. + """ + deps = manifest.get("python_dependencies") or [] + if not isinstance(deps, list): + return + deps = [d.strip() for d in deps if isinstance(d, str) and d.strip()] + if not deps: + return + plugin_name = manifest.get("name", "this plugin") + console.print( + f"\n[bold]{plugin_name}[/bold] declares Python dependencies " + "(not installed automatically):" + ) + for dep in deps: + console.print(f" - {dep}") + console.print( + "[dim]Install them yourself if needed: " + f"pip install {' '.join(repr(d) for d in deps)}[/dim]\n" + ) + + def _prompt_plugin_env_vars(manifest: dict, console) -> None: """Prompt for required environment variables declared in plugin.yaml. @@ -835,6 +861,8 @@ def cmd_install( _prompt_plugin_env_vars(installed_manifest, console) + _print_python_dependencies(installed_manifest, console) + _display_after_install(target, identifier) should_enable = enable diff --git a/tests/hermes_cli/test_plugin_manifest_v2.py b/tests/hermes_cli/test_plugin_manifest_v2.py new file mode 100644 index 0000000000..3e8a0d9d9f --- /dev/null +++ b/tests/hermes_cli/test_plugin_manifest_v2.py @@ -0,0 +1,411 @@ +"""Tests for plugin manifest v2 (#64165). + +Covers: v1 regression (unchanged behavior), v2 field parsing, unknown-field +forward compat, requires_plugins load ordering + cycle handling, +config_schema validation warnings, and the python_dependencies +declare-only seam (surfaced, never installed). +""" + +import logging + +import pytest +import yaml + +from hermes_cli.plugins import ( + PluginManager, + PluginManifest, + SUPPORTED_MANIFEST_VERSION, + resolve_plugin_load_order, + validate_config_schema, +) + + +def _write_plugin(base, name, manifest_extra=None, register_body="pass"): + plugin_dir = base / name + plugin_dir.mkdir(parents=True, exist_ok=True) + manifest = {"name": name, "version": "0.1.0", "description": f"test {name}"} + if manifest_extra: + manifest.update(manifest_extra) + (plugin_dir / "plugin.yaml").write_text(yaml.dump(manifest)) + (plugin_dir / "__init__.py").write_text( + f"def register(ctx):\n {register_body}\n" + ) + return plugin_dir + + +def _enable(home, names, entries=None): + cfg = {"plugins": {"enabled": list(names)}} + if entries: + cfg["plugins"]["entries"] = entries + (home / "config.yaml").write_text(yaml.safe_dump(cfg)) + + +@pytest.fixture +def hermes_home(tmp_path, monkeypatch): + home = tmp_path / "hermes_home" + (home / "plugins").mkdir(parents=True) + monkeypatch.setenv("HERMES_HOME", str(home)) + monkeypatch.setenv("HERMES_ENABLE_PROJECT_PLUGINS", "0") + monkeypatch.setenv( + "HERMES_BUNDLED_PLUGINS", str(tmp_path / "empty-bundled") + ) + (tmp_path / "empty-bundled").mkdir() + return home + + +class TestV1Regression: + def test_v1_manifest_parses_with_defaults(self, hermes_home): + _write_plugin(hermes_home / "plugins", "oldie") + _enable(hermes_home, ["oldie"]) + mgr = PluginManager() + mgr.discover_and_load() + loaded = mgr._plugins["oldie"] + assert loaded.enabled + m = loaded.manifest + assert m.manifest_version == 1 + assert m.api_version is None + assert m.requires_plugins == [] + assert m.python_dependencies == [] + assert m.config_schema == {} + assert m.license == "" + assert m.homepage == "" + assert m.tags == [] + + def test_v1_unknown_fields_do_not_warn_loudly(self, hermes_home, caplog): + _write_plugin( + hermes_home / "plugins", "oldie", + manifest_extra={"mystery_field": True}, + ) + _enable(hermes_home, ["oldie"]) + with caplog.at_level(logging.WARNING, logger="hermes_cli.plugins"): + mgr = PluginManager() + mgr.discover_and_load() + assert mgr._plugins["oldie"].enabled + assert "mystery_field" not in caplog.text + + +class TestV2Parsing: + def test_v2_fields_parse(self, hermes_home): + _write_plugin( + hermes_home / "plugins", "modern", + manifest_extra={ + "manifest_version": 2, + "api_version": 1, + "license": "MIT", + "homepage": "https://example.com/modern", + "tags": ["gateway", "demo"], + "requires_plugins": [ + {"id": "other", "version_range": ">=1.0,<2"}, + "bare-dep", + ], + "python_dependencies": ["requests>=2.0,<3"], + "config_schema": { + "api_url": {"type": "str", "default": "", "description": "x"}, + }, + }, + ) + _enable(hermes_home, ["modern"]) + mgr = PluginManager() + mgr.discover_and_load() + m = mgr._plugins["modern"].manifest + assert m.manifest_version == 2 + assert m.api_version == 1 + assert m.license == "MIT" + assert m.homepage == "https://example.com/modern" + assert m.tags == ["gateway", "demo"] + assert m.requires_plugins == [ + {"id": "other", "version_range": ">=1.0,<2"}, + {"id": "bare-dep", "version_range": None}, + ] + assert m.python_dependencies == ["requests>=2.0,<3"] + assert "api_url" in m.config_schema + # plugin still loads + assert mgr._plugins["modern"].enabled or mgr._plugins["modern"].error + + def test_unknown_field_in_v2_warns_but_loads(self, hermes_home, caplog): + _write_plugin( + hermes_home / "plugins", "modern", + manifest_extra={"manifest_version": 2, "hovercraft": "eels"}, + ) + _enable(hermes_home, ["modern"]) + with caplog.at_level(logging.WARNING): + mgr = PluginManager() + mgr.discover_and_load() + assert mgr._plugins["modern"].enabled + assert "hovercraft" in caplog.text + + def test_future_manifest_version_warns_but_loads(self, hermes_home, caplog): + _write_plugin( + hermes_home / "plugins", "fromfuture", + manifest_extra={ + "manifest_version": SUPPORTED_MANIFEST_VERSION + 5, + }, + ) + _enable(hermes_home, ["fromfuture"]) + with caplog.at_level(logging.WARNING): + mgr = PluginManager() + mgr.discover_and_load() + assert mgr._plugins["fromfuture"].enabled + assert "newer than this Hermes" in caplog.text + + def test_malformed_v2_fields_warn_and_degrade(self, hermes_home, caplog): + _write_plugin( + hermes_home / "plugins", "sloppy", + manifest_extra={ + "manifest_version": 2, + "api_version": "banana", + "requires_plugins": "not-a-list", + "python_dependencies": {"nope": 1}, + "tags": "not-a-list", + }, + ) + _enable(hermes_home, ["sloppy"]) + with caplog.at_level(logging.WARNING): + mgr = PluginManager() + mgr.discover_and_load() + loaded = mgr._plugins["sloppy"] + assert loaded.enabled + m = loaded.manifest + assert m.api_version is None + assert m.requires_plugins == [] + assert m.python_dependencies == [] + assert m.tags == [] + + +class TestDependencyOrder: + def test_dep_registers_before_dependent(self, hermes_home): + # zzz-consumer requires aaa-base... but alphabetically consumer + # would load AFTER base anyway, so invert: aaa-consumer requires + # zzz-base, forcing the topo sort to override alpha order. + _write_plugin( + hermes_home / "plugins", "aaa-consumer", + manifest_extra={ + "manifest_version": 2, + "requires_plugins": [{"id": "zzz-base"}], + }, + register_body="import sys; sys._m2_order.append('aaa-consumer')", + ) + _write_plugin( + hermes_home / "plugins", "zzz-base", + register_body="import sys; sys._m2_order.append('zzz-base')", + ) + _enable(hermes_home, ["aaa-consumer", "zzz-base"]) + import sys + + sys._m2_order = [] + try: + mgr = PluginManager() + mgr.discover_and_load() + assert sys._m2_order == ["zzz-base", "aaa-consumer"] + finally: + del sys._m2_order + + def test_missing_dep_warns_but_loads(self, hermes_home, caplog): + _write_plugin( + hermes_home / "plugins", "needy", + manifest_extra={ + "manifest_version": 2, + "requires_plugins": [{"id": "ghost-plugin"}], + }, + ) + _enable(hermes_home, ["needy"]) + with caplog.at_level(logging.WARNING): + mgr = PluginManager() + mgr.discover_and_load() + assert mgr._plugins["needy"].enabled + assert "ghost-plugin" in caplog.text + assert "loading anyway" in caplog.text + + def test_cycle_warns_and_falls_back_alpha(self, hermes_home, caplog): + _write_plugin( + hermes_home / "plugins", "cyc-a", + manifest_extra={ + "manifest_version": 2, + "requires_plugins": [{"id": "cyc-b"}], + }, + register_body="import sys; sys._m2_cycle.append('cyc-a')", + ) + _write_plugin( + hermes_home / "plugins", "cyc-b", + manifest_extra={ + "manifest_version": 2, + "requires_plugins": [{"id": "cyc-a"}], + }, + register_body="import sys; sys._m2_cycle.append('cyc-b')", + ) + _enable(hermes_home, ["cyc-a", "cyc-b"]) + import sys + + sys._m2_cycle = [] + try: + with caplog.at_level(logging.WARNING): + mgr = PluginManager() + mgr.discover_and_load() + # Both still load, in alphabetical fallback order. + assert sys._m2_cycle == ["cyc-a", "cyc-b"] + assert "cycle" in caplog.text.lower() + finally: + del sys._m2_cycle + + def test_resolve_order_pure_function(self): + manifests = { + "b": PluginManifest(name="b", key="b", + requires_plugins=[{"id": "c"}]), + "a": PluginManifest(name="a", key="a", + requires_plugins=[{"id": "b"}]), + "c": PluginManifest(name="c", key="c"), + } + assert resolve_plugin_load_order(manifests) == ["c", "b", "a"] + + def test_resolve_order_matches_by_manifest_name(self): + manifests = { + "cat/impl": PluginManifest( + name="impl-name", key="cat/impl" + ), + "user": PluginManifest( + name="user", key="user", + requires_plugins=[{"id": "impl-name"}], + ), + } + assert resolve_plugin_load_order(manifests) == ["cat/impl", "user"] + + +class TestConfigSchema: + def test_type_mismatch_warns_but_loads(self, hermes_home, caplog): + _write_plugin( + hermes_home / "plugins", "cfgd", + manifest_extra={ + "manifest_version": 2, + "config_schema": { + "api_url": {"type": "str"}, + "retries": {"type": "int"}, + }, + }, + ) + _enable( + hermes_home, ["cfgd"], + entries={"cfgd": {"settings": {"api_url": 42, "retries": 3}}}, + ) + with caplog.at_level(logging.WARNING): + mgr = PluginManager() + mgr.discover_and_load() + assert mgr._plugins["cfgd"].enabled + assert "plugins.entries.cfgd.settings.api_url" in caplog.text + assert "should be str" in caplog.text + assert "retries" not in caplog.text.split("should be")[-1] + + def test_required_key_missing_warns(self): + warnings = validate_config_schema( + "p", {"token": {"type": "str", "required": True}}, {} + ) + assert warnings and "required" in warnings[0] + assert "plugins.entries.p.settings.token" in warnings[0] + + def test_valid_settings_produce_no_warnings(self): + schema = { + "api_url": {"type": "str"}, + "retries": {"type": "int"}, + "ratio": {"type": "float"}, + "flag": {"type": "bool"}, + } + settings = {"api_url": "x", "retries": 2, "ratio": 0.5, "flag": True} + assert validate_config_schema("p", schema, settings) == [] + + def test_bool_does_not_satisfy_int(self): + warnings = validate_config_schema( + "p", {"retries": {"type": "int"}}, {"retries": True} + ) + assert warnings and "should be int" in warnings[0] + + def test_unknown_declared_type_skips_check(self, hermes_home, caplog): + _write_plugin( + hermes_home / "plugins", "weird", + manifest_extra={ + "manifest_version": 2, + "config_schema": {"thing": {"type": "quaternion"}}, + }, + ) + _enable( + hermes_home, ["weird"], + entries={"weird": {"settings": {"thing": 1}}}, + ) + with caplog.at_level(logging.WARNING): + mgr = PluginManager() + mgr.discover_and_load() + assert mgr._plugins["weird"].enabled + assert "quaternion" in caplog.text + assert "should be" not in caplog.text + + +class TestPythonDependenciesSeam: + def test_missing_pip_dep_surfaced_with_hint_not_installed( + self, hermes_home, caplog, monkeypatch + ): + calls = [] + import subprocess + + def _spy_run(*args, **kwargs): + calls.append(args) + raise AssertionError("no subprocess should run for pip deps") + + monkeypatch.setattr(subprocess, "run", _spy_run) + monkeypatch.setattr(subprocess, "check_call", _spy_run) + _write_plugin( + hermes_home / "plugins", "pipful", + manifest_extra={ + "manifest_version": 2, + "python_dependencies": [ + "definitely-not-a-real-package-64165>=1.0,<2", + ], + }, + ) + _enable(hermes_home, ["pipful"]) + with caplog.at_level(logging.WARNING): + mgr = PluginManager() + mgr.discover_and_load() + assert mgr._plugins["pipful"].enabled + assert "definitely-not-a-real-package-64165" in caplog.text + assert "pip install" in caplog.text + assert "does not install plugin dependencies automatically" in caplog.text + assert calls == [] + + def test_satisfied_pip_dep_is_quiet(self, hermes_home, caplog): + _write_plugin( + hermes_home / "plugins", "pipok", + manifest_extra={ + "manifest_version": 2, + "python_dependencies": ["pyyaml>=5,<7"], + }, + ) + _enable(hermes_home, ["pipok"]) + with caplog.at_level(logging.WARNING): + mgr = PluginManager() + mgr.discover_and_load() + assert mgr._plugins["pipok"].enabled + assert "pip install" not in caplog.text + + +class TestCtxHasPlugin: + def test_has_plugin_probe(self, hermes_home): + _write_plugin(hermes_home / "plugins", "probe-target") + _write_plugin( + hermes_home / "plugins", "prober", + manifest_extra={ + "manifest_version": 2, + "requires_plugins": [{"id": "probe-target"}], + }, + register_body=( + "import sys; sys._m2_probe = (" + "ctx.has_plugin('probe-target'), ctx.has_plugin('nope'))" + ), + ) + _enable(hermes_home, ["probe-target", "prober"]) + import sys + + try: + mgr = PluginManager() + mgr.discover_and_load() + assert sys._m2_probe == (True, False) + finally: + if hasattr(sys, "_m2_probe"): + del sys._m2_probe diff --git a/website/docs/developer-guide/plugins/index.md b/website/docs/developer-guide/plugins/index.md index ec963db188..ad241b350f 100644 --- a/website/docs/developer-guide/plugins/index.md +++ b/website/docs/developer-guide/plugins/index.md @@ -253,6 +253,53 @@ config keys (`plugins.entries..allow_tool_override`, …) still work but are deprecated — declare capabilities instead so users get a single, auditable consent screen. Capabilities are consent + audit, **not a sandbox**: they gate host API surfaces, nothing more. +### Manifest v2 reference + +`plugin.yaml` also supports an additive **v2 schema** (#64165). Every field is +optional; a manifest without `manifest_version` is a v1 manifest and stays +fully supported forever. Unknown fields never break loading — they are ignored +with a warning (forward compatibility), and a `manifest_version` newer than +this Hermes understands still loads with a warning. + +| Field | Type | Meaning | +|---|---|---| +| `manifest_version` | int | Manifest **file-format** version. Absent = `1`. Current max: `2`. Independent from `api_version`. | +| `api_version` | int | Runtime **plugin API generation** the plugin targets (ctx surface / hook signatures). Deliberately a separate axis from `manifest_version` — an `api_version: 1` plugin can use a v2 manifest. | +| `requires_plugins` | list | Inter-plugin dependencies: `- id: other-plugin` with optional `version_range: ">=1.0,<2"`. **Advisory**: a missing dependency logs a clear warning but the plugin still loads — probe at runtime with `ctx.has_plugin("other-plugin")`. Load **order** honors these edges: when A requires B, B's `register()` runs before A's (topological sort, alphabetical tiebreak; cycles warn and fall back to alphabetical order). | +| `python_dependencies` | list of str | Declared pip requirements (e.g. `"requests>=2.0,<3"`). **Declaration seam only** — Hermes validates them, and `hermes plugins install` / `hermes plugins doctor` surface missing ones with a `pip install` hint, but Hermes **never auto-installs** them. Pin upper bounds. | +| `config_schema` | mapping | JSON-schema-ish description of keys under `plugins.entries..settings`: `api_url: {type: str, default: "", description: "...", required: false}`. Validated at load; mismatches log actionable warnings naming the key and expected type — never load failures. Types: `str`, `int`, `float`, `bool`, `list`, `dict` (plus JSON-schema aliases). | +| `license` | str | SPDX-style license id (e.g. `MIT`). | +| `homepage` | str | Project URL. | +| `tags` | list of str | Free-form discovery tags (e.g. `[gateway, telegram]`). | + +```yaml +# plugin.yaml — manifest v2 example +name: my-plugin +version: 1.2.0 +manifest_version: 2 +api_version: 1 +license: MIT +homepage: https://github.com/owner/my-plugin +tags: [gateway, demo] +requires_plugins: + - id: other-plugin + version_range: ">=1.0,<2" +python_dependencies: + - "somepkg>=1.0,<2" # surfaced, never auto-installed +config_schema: + api_url: {type: str, default: "", description: "Service endpoint"} +``` + +:::note pip-dependency isolation is deferred +`python_dependencies` is intentionally declare-and-surface only. Installing +arbitrary packages into Hermes' shared venv is a conflict and supply-chain +surface, so the install seam's isolation design (constraints-file installs +against the host lock vs. per-plugin vendored dirs vs. conflict detection +with refusal) is an explicitly deferred follow-up — see the round-2 review on +[#64165](https://github.com/NousResearch/hermes-agent/issues/64165) and +[#15220](https://github.com/NousResearch/hermes-agent/issues/15220). Plugin +packs (#64166) build on these v2 fields. +::: ## Step 3: Write the tool schemas