From 6ac3ff8867cd97621944defd8b39e10c10f07b2b Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Wed, 16 Sep 2026 22:56:59 -0700 Subject: [PATCH] feat(plugins validate): fail trees with nothing to load; check dependency declarations A plugin.yaml with no __init__.py, desktop/plugin.js or plugin.json beside it installs "successfully" and does nothing (pip-layout repos whose code sits under src/ behind an entry point). The validator and catalog CI now fail that shape and check declared Python dependencies parse and are index specs; direct-URL requirements warn. Tests: conflicting candidate refused with peers untouched; post-update re-apply drops only the culprit and keeps the memory provider. Loader wording test updated to the new hint. --- hermes_cli/plugin_validate.py | 50 ++++++++++++++++++++ tests/hermes_cli/test_plugin_manifest_v2.py | 2 +- tests/hermes_cli/test_plugin_python_deps.py | 11 +++-- tests/hermes_cli/test_plugins_cmd_catalog.py | 3 +- 4 files changed, 59 insertions(+), 7 deletions(-) diff --git a/hermes_cli/plugin_validate.py b/hermes_cli/plugin_validate.py index 41933785a2..77d1f04f7c 100644 --- a/hermes_cli/plugin_validate.py +++ b/hermes_cli/plugin_validate.py @@ -508,12 +508,62 @@ def validate_plugin_dir(plugin_dir: Path) -> ValidationReport: _check_requires_hermes(report, manifest) _check_config_spec(report, manifest) _check_requires_env(report, manifest) + _check_loadable(report, plugin_dir) + _check_python_dependencies(report, plugin_dir) recorded = _check_capabilities(report, manifest, plugin_dir) _check_builtin_collisions(report, manifest, recorded) _check_security_scan(report, plugin_dir) return report +_LOADABLE_ENTRYPOINTS = ("__init__.py", "desktop/plugin.js", "plugin.json") + + +def _check_loadable(report: ValidationReport, plugin_dir: Path) -> None: + """A plugin.yaml with nothing beside it that Hermes can load (no ``register()`` module, no + desktop bundle, no portable manifest) installs "successfully" and does nothing — a pip-layout + repo whose code lives under ``src/`` behind an entry point is the usual shape.""" + present = [rel for rel in _LOADABLE_ENTRYPOINTS if (plugin_dir / rel).is_file()] + report.add( + "loadable", bool(present), + f"entry: {', '.join(present)}" if present else + "nothing to load: no __init__.py, desktop/plugin.js or plugin.json beside plugin.yaml " + "(pip-layout packages need a directory-plugin wrapper with a pyproject.toml declaring the deps)", + ) + + +def _check_python_dependencies(report: ValidationReport, plugin_dir: Path) -> None: + """Declared deps (pyproject ``[project].dependencies`` or manifest ``python_dependencies``) must be + well-formed PEP 508 specs the installer will accept; a plugin opting out with + ``python_runtime: external`` declares none.""" + from hermes_cli.plugin_python_deps import read_declaration + + try: + decl = read_declaration(plugin_dir) + except Exception as exc: + report.add("python dependencies", False, f"declaration invalid: {exc}") + return + if decl.external: + report.add("python dependencies", True, "external runtime (plugin manages its own)") + return + from hermes_cli.plugin_python_deps import applicable_specs, unsupported_specs + + urls = unsupported_specs(decl.specs) + if urls: + report.warn("python dependencies: direct URL requirement(s) are never auto-installed, users must " + f"install them by hand: {', '.join(urls)}") + installable = applicable_specs(decl.specs) + rejected = [s for s in installable if not _spec_is_safe(s)] + detail = f"{len(installable)} installable from {decl.source}" if decl.source else "none declared" + report.add("python dependencies", not rejected, + f"unsafe spec(s): {', '.join(rejected)}" if rejected else detail) + + +def _spec_is_safe(spec: str) -> bool: + from tools.lazy_deps import _spec_is_safe as safe + return safe(spec) + + def _check_security_scan(report: ValidationReport, plugin_dir: Path) -> None: """Run the install-time scanner at admission, so a pin a reviewer approves is one the installer will accept: ``dangerous`` fails the entry; ``caution`` findings surface as diff --git a/tests/hermes_cli/test_plugin_manifest_v2.py b/tests/hermes_cli/test_plugin_manifest_v2.py index a38e9a328e..5a8b0d900e 100644 --- a/tests/hermes_cli/test_plugin_manifest_v2.py +++ b/tests/hermes_cli/test_plugin_manifest_v2.py @@ -366,7 +366,7 @@ class TestPythonDependenciesSeam: 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 "hermes plugins enable pipful" in caplog.text assert calls == [] def test_satisfied_pip_dep_is_quiet(self, hermes_home, caplog): diff --git a/tests/hermes_cli/test_plugin_python_deps.py b/tests/hermes_cli/test_plugin_python_deps.py index 1e2c34f78e..ff26cc995e 100644 --- a/tests/hermes_cli/test_plugin_python_deps.py +++ b/tests/hermes_cli/test_plugin_python_deps.py @@ -53,32 +53,33 @@ def test_conflicting_candidate_is_refused_and_enabled_plugins_are_untouched(tmp_ resolve, calls = _fake_resolver({"tabulate<0.9"}) monkeypatch.setattr(deps, "resolve", resolve) monkeypatch.setattr(deps, "core_constraints", lambda root: ["httpx==0.28.1"]) - monkeypatch.setattr(deps, "all_homes", lambda: [home]) + monkeypatch.setattr(deps, "dependency_homes", lambda: [home]) with pytest.raises(deps.DependencyConflict): deps.check_candidate(deps.read_declaration(candidate), home=home, project_root=tmp_path) # The dry run saw the union (candidate + enabled peer) and never ran a real install. - assert calls == [["tabulate<0.9", "tabulate>=0.9"]] + assert [sorted(c) for c in calls] == [["tabulate<0.9", "tabulate>=0.9"]] assert (home / "plugins" / "good").is_dir() assert "good" in (home / "config.yaml").read_text() def test_reapply_disables_non_memory_plugin_loudly_and_keeps_memory_provider(tmp_path, monkeypatch): - home = _home(tmp_path, ["memory", "weather", "sidecar"]) + home = _home(tmp_path, ["memory", "weather", "sidecar", "aaa-innocent"]) _plugin(home, "memory", '"mnemosyne-memory>=3"', memory=True) _plugin(home, "weather", '"httpx<0.20"') _plugin(home, "sidecar", '"torch==99"', external=True) + _plugin(home, "aaa-innocent", '"tabulate>=0.9"') # sorts before the culprit; must NOT be sacrificed resolve, calls = _fake_resolver({"httpx<0.20"}) monkeypatch.setattr(deps, "resolve", resolve) monkeypatch.setattr(deps, "core_constraints", lambda root: ["httpx==0.28.1"]) - monkeypatch.setattr(deps, "all_homes", lambda: [home]) + monkeypatch.setattr(deps, "dependency_homes", lambda: [home]) disabled: list[tuple[Path, str]] = [] report = deps.reapply_all(project_root=tmp_path, disable=lambda h, n: disabled.append((h, n))) assert disabled == [(home, "weather")] assert report.dropped and report.dropped[0][0] == "weather" - assert report.installed == ["mnemosyne-memory>=3"] + assert sorted(report.installed) == ["mnemosyne-memory>=3", "tabulate>=0.9"] # External-runtime plugin never joined the union; memory provider was the survivor. assert all("torch==99" not in c for c in calls) diff --git a/tests/hermes_cli/test_plugins_cmd_catalog.py b/tests/hermes_cli/test_plugins_cmd_catalog.py index f42f1d0165..2333549792 100644 --- a/tests/hermes_cli/test_plugins_cmd_catalog.py +++ b/tests/hermes_cli/test_plugins_cmd_catalog.py @@ -76,7 +76,8 @@ def test_catalog_name_installs_pinned_sha_with_sidecar_then_update_repins(world, # Dashboard update on a catalog install = re-pin. Pin unchanged → no-op. assert pc.dashboard_update_user_plugin("cat-plugin") == { - "ok": True, "name": "cat-plugin", "sha": world["sha1"], "unchanged": True} + "ok": True, "name": "cat-plugin", "sha": world["sha1"], "unchanged": True, + "python_dependencies": [], "warnings": []} # Bump the catalog pin → the checkout moves to exactly that sha. world["state"]["pin"] = world["sha2"] assert pc.dashboard_update_user_plugin("cat-plugin")["unchanged"] is False