diff --git a/steward/core/plugin_manager.py b/steward/core/plugin_manager.py index 981c95b..603136d 100644 --- a/steward/core/plugin_manager.py +++ b/steward/core/plugin_manager.py @@ -305,16 +305,29 @@ def load_plugins(app: "Quart") -> None: def hot_reload_plugin(app: "Quart", name: str) -> tuple[bool, str]: """Activate a plugin that was just enabled without restarting the app. - This works for plugins that were NOT previously loaded. For plugins that - are already registered (blueprint already mounted), a full restart is - required to pick up code changes — use restart_app() in that case. + A plugin that can't be loaded live is recorded with its reason, as a + failed boot load is, so its settings page says why and offers a restart + rather than reading "Enabled, not loaded" with nothing more (#6087). Returns (success, message). """ + ok, message = _hot_load(app, name) + if ok: + _FAILED_PLUGINS.pop(name, None) + else: + _FAILED_PLUGINS[name] = message + logger.warning("Plugin %r could not be loaded live: %s", name, message) + return ok, message + + +def _hot_load(app: "Quart", name: str) -> tuple[bool, str]: + """Load one plugin into the running app. A plugin already loaded (enabled, + disabled and enabled again without a restart) is still running, so that + is a success; picking up its code changes needs restart_app().""" import steward if name in _LOADED_PLUGINS: - return False, "Plugin already loaded — restart required to apply updates" + return True, "Plugin already running" plugin_dirs = [Path(d).resolve() for d in app.config["PLUGIN_DIRS"]] plugin_path = resolve_plugin_path(plugin_dirs, name) @@ -373,9 +386,9 @@ def hot_reload_plugin(app: "Quart", name: str) -> tuple[bool, str]: ) all_dirs = discover_all_in(plugin_dirs) run_plugin_migrations(app.config["DATABASE_URL"], all_dirs) - except Exception: + except Exception as exc: logger.exception("Plugin %r: migration failed during hot-reload", name) - return False, "Migration failed — check logs" + return False, f"Migration failed: {exc}" # Import (file-path load avoids stdlib name collisions) try: diff --git a/steward/settings/routes.py b/steward/settings/routes.py index e8a5ce0..221a8be 100644 --- a/steward/settings/routes.py +++ b/steward/settings/routes.py @@ -1091,8 +1091,11 @@ async def save_plugin_detail(name: str): action = "plugin.enabled" if new_enabled else "plugin.disabled" await log_audit(current_app, session.get("user_id"), session.get("username", ""), action, entity_type="plugin", entity_id=name) - if new_enabled and not old_enabled: - from steward.core.plugin_manager import hot_reload_plugin + # Any save of an enabled plugin that isn't running tries to start it; a + # failure is recorded with its reason, which the page shows beside a + # restart (#6087). + from steward.core.plugin_manager import _LOADED_PLUGINS, hot_reload_plugin + if new_enabled and name not in _LOADED_PLUGINS: hot_reload_plugin(current_app._get_current_object(), name) return redirect(url_for("settings.plugin_detail", name=name)) diff --git a/steward/templates/settings/plugin_detail.html b/steward/templates/settings/plugin_detail.html index 52f3dce..034e6f6 100644 --- a/steward/templates/settings/plugin_detail.html +++ b/steward/templates/settings/plugin_detail.html @@ -37,6 +37,20 @@ {% endif %} +{% if plugin._enabled and not is_loaded and not plugin._always_on %} +
+{% endif %} + {% if plugin.get('description') %}{{ plugin.description }}
{% endif %} diff --git a/tests/core/test_plugin_hot_load.py b/tests/core/test_plugin_hot_load.py new file mode 100644 index 0000000..d2cebe1 --- /dev/null +++ b/tests/core/test_plugin_hot_load.py @@ -0,0 +1,64 @@ +"""A plugin enabled while Steward runs is loaded live; one that can't be is +recorded with its reason, so its settings page says why and offers a restart +instead of a bare "Enabled, not loaded" (#6087).""" +from __future__ import annotations + +import sys +from types import SimpleNamespace + +import pytest + +from steward.core import plugin_manager as pm + + +@pytest.fixture(autouse=True) +def clean_registries(monkeypatch): + monkeypatch.setattr(pm, "_LOADED_PLUGINS", set()) + monkeypatch.setattr(pm, "_FAILED_PLUGINS", {}) + monkeypatch.setattr(pm, "_PLUGIN_NAV", []) + monkeypatch.setattr(pm, "_PLUGIN_MODULES", {}) + yield + for key in [k for k in sys.modules if k.startswith("_steward_plugin_hl_")]: + sys.modules.pop(key) + + +def _app(root): + return SimpleNamespace(config={"PLUGIN_DIRS": [str(root)], "PLUGINS": {}}, + _task_registry=[], mcp_server=None) + + +def _plugin(root, name, *, yaml_name=None, setup_body="pass"): + pdir = root / name + pdir.mkdir(parents=True) + (pdir / "plugin.yaml").write_text(f"name: {yaml_name or name}\nversion: 1.0.0\n") + (pdir / "__init__.py").write_text( + f"def setup(app):\n {setup_body}\n\n" + "def get_scheduled_tasks():\n return []\n") + + +def test_a_plugin_that_cannot_load_live_is_recorded_with_its_reason(tmp_path): + _plugin(tmp_path, "hl_broken", setup_body="raise RuntimeError('no controller')") + ok, message = pm.hot_reload_plugin(_app(tmp_path), "hl_broken") + assert not ok + assert pm.get_plugin_failures() == {"hl_broken": "setup() raised: no controller"} + assert "hl_broken" not in pm._LOADED_PLUGINS + + +def test_an_early_refusal_is_recorded_too(tmp_path): + _plugin(tmp_path, "hl_misnamed", yaml_name="other") + ok, _ = pm.hot_reload_plugin(_app(tmp_path), "hl_misnamed") + assert not ok and "name mismatch" in pm.get_plugin_failures()["hl_misnamed"] + + +def test_a_plugin_that_loads_clears_an_earlier_failure(tmp_path): + _plugin(tmp_path, "hl_good") + pm._FAILED_PLUGINS["hl_good"] = "setup() raised: earlier" + ok, _ = pm.hot_reload_plugin(_app(tmp_path), "hl_good") + assert ok + assert "hl_good" in pm._LOADED_PLUGINS and pm.get_plugin_failures() == {} + + +def test_enabling_one_already_running_is_a_success_not_a_failure(tmp_path): + pm._LOADED_PLUGINS.add("hl_running") + ok, _ = pm.hot_reload_plugin(_app(tmp_path), "hl_running") + assert ok and pm.get_plugin_failures() == {}