From 9ef9a8bfa7b0519208fddfcaa7ae2577931ca5c5 Mon Sep 17 00:00:00 2001 From: Peter Szabados Date: Sat, 11 Jul 2026 09:37:47 +0200 Subject: [PATCH 1/5] Remove deleted app from memory --- appdaemon/app_management.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/appdaemon/app_management.py b/appdaemon/app_management.py index 729c1fad0..eb0768538 100644 --- a/appdaemon/app_management.py +++ b/appdaemon/app_management.py @@ -496,7 +496,7 @@ async def stop_app(self, app_name: str, *, delete: bool = False) -> bool: bool: Whether stopping was successful or not """ try: - if isinstance(self.app_config[app_name], AppConfig): + if isinstance(self.app_config.root.get(app_name), AppConfig): self.logger.debug("Stopping app '%s'", app_name) await self.terminate_app(app_name, delete=delete) except Exception: @@ -780,7 +780,7 @@ async def check_app_config_files(self, update_actions: UpdateActions): ) # fmt: skip update_actions.apps.term |= deleted_apps for name in deleted_apps: - # del self.app_config.root[name] + self.app_config.root.pop(name, None) self.logger.info("App config deleted: %s", name) self.app_config.root.update(freshly_read_cfg.root) @@ -1208,7 +1208,7 @@ async def _stop_apps(self, update_actions: UpdateActions): failed_to_stop = set() # stores apps that had a problem terminating for app_name in stop_order: - successfully_stopped = await self.stop_app(app_name) + successfully_stopped = await self.stop_app(app_name, delete=True) if successfully_stopped: self.logger.info("Stopped app '%s'", app_name) if app_name in indirect_stops: From 187e915f09f2a1b142e65a65e6fb889f97a6df5c Mon Sep 17 00:00:00 2001 From: Peter Szabados Date: Sun, 4 Oct 2026 22:40:32 +0200 Subject: [PATCH 2/5] Keep apps stopped while a plugin is down When a plugin (e.g. HASS) fails, PLUGIN_FAILED stops and deletes all of its apps' ManagedObjects. The utility loop's periodic NORMAL check_app_updates() pass then sees every app as "not in objects" and re-adds them to init, prematurely restarting all apps against the dead plugin (spurious restart wave; apps crash in initialize with connection errors). The apps should stay stopped until the plugin reconnects and PLUGIN_RESTART runs. - Add PluginManagement.any_plugin_inactive() and skip the not-in-objects init repopulation in check_app_config_files while any plugin is marked inactive. The guard covers only that repopulation; config-file-change loads are intentionally not guarded. - Record the stopped app names per namespace when a plugin fails (AppManagement.stopped_plugin_apps) and restore them in _start_plugin_apps on PLUGIN_RESTART, since the deleted ManagedObjects no longer carry the namespace. Snapshot entries are filtered at pop time to configs that still exist and are not disabled. - Register plugins as active from the start via a _register_plugin() seam (also seeds last_plugin_state). The active flag is now purely a runtime failure signal: a never-connected plugin no longer counts as inactive, so the initial app load can't hit the guard at boot (with production_mode the utility loop never re-runs check_app_updates, so a boot-time guard hit would leave no apps running). - Make HassPlugin.stop() tolerate a never-connected plugin (ws/session are None until a websocket connect succeeds) and wrap per-plugin stop calls in PluginManagement.stop() so one failing plugin cannot abort the whole shutdown sequence. Fixes the HASS-restart app storm (wave 1: 126 apps spuriously started, 27 AppStartFailures, double restart per HASS restart). --- appdaemon/app_management.py | 40 ++- appdaemon/plugin_management.py | 46 +++- appdaemon/plugins/hass/hassplugin.py | 30 ++- tests/functional/test_plugin_guard.py | 335 ++++++++++++++++++++++++++ 4 files changed, 430 insertions(+), 21 deletions(-) create mode 100644 tests/functional/test_plugin_guard.py diff --git a/appdaemon/app_management.py b/appdaemon/app_management.py index eb0768538..a3761d90c 100644 --- a/appdaemon/app_management.py +++ b/appdaemon/app_management.py @@ -90,6 +90,9 @@ def __init__(self, ad: "AppDaemon"): self.diag = ad.logging.get_diag() self.filter_files = {} self.objects = {} + # Namespace -> apps that were stopped when the plugin failed. Used by _start_plugin_apps + # to restore them, since their ManagedObjects are deleted and no longer carry the namespace + self.stopped_plugin_apps: dict[str, set[str]] = {} # Add Path for adbase sys.path.insert(0, os.path.dirname(__file__)) @@ -789,13 +792,21 @@ async def check_app_config_files(self, update_actions: UpdateActions): # If there are any new/modified apps, the dependency graph needs to be updated self.dependency_manager.app_deps.refresh_dep_graph() - update_actions.apps.init |= { - name for name, cfg in self.app_config.root.items() - if name not in self.objects - and isinstance(cfg, AppConfig) - and not cfg.disable - and await self.get_state(name) != "compile_error" - } # fmt: skip + if self.AD.plugins.any_plugin_inactive(): + # Don't auto-restart apps while a plugin is down - they'll be started + # again when the plugin reconnects (PLUGIN_RESTART) + # Scope: this guard covers only the repopulation of apps that aren't in self.objects. + # Loads initiated by an explicit config-file change (init/reload adds above) are NOT + # guarded — an explicit user action during a plugin outage is honored. + self.logger.debug("Skipping init repopulation because a plugin is inactive") + else: + update_actions.apps.init |= { + name for name, cfg in self.app_config.root.items() + if name not in self.objects + and isinstance(cfg, AppConfig) + and not cfg.disable + and await self.get_state(name) != "compile_error" + } # fmt: skip @executor_decorator def read_config_file(self, file: Path) -> AllAppConfig: @@ -1177,9 +1188,15 @@ def get_namespace_apps(self, namespace: str) -> set[str]: ) # fmt: skip async def _stop_plugin_apps(self, plugin_ns: str | None, update_actions: UpdateActions): + """Stops the apps running in the namespace of a plugin that just failed, so they can be + restored by :py:meth:`~_start_plugin_apps` when the plugin reconnects.""" if plugin_ns is not None: self.logger.info(f"Stopping apps from namespace '{plugin_ns}' because the plugin failed") app_names = self.get_namespace_apps(plugin_ns) + # Snapshot the app names before they're stopped and their ManagedObjects deleted, so + # _start_plugin_apps can restore them when the plugin comes back + snapshot = self.stopped_plugin_apps.setdefault(plugin_ns, set()) + snapshot |= app_names deps = self.dependency_manager.app_deps.get_dependents(app_names) update_actions.apps.term |= deps @@ -1187,10 +1204,19 @@ async def _start_plugin_apps(self, plugin_ns: str | None, update_actions: Update """If a plugin ever re-connects after the initial startup, the apps that use it's plugin all need to be started. They should already have been stopped by the plugin disconnecting. The apps that belong to the plugin are determined by namespace. + + Apps whose ManagedObjects were deleted by the plugin failure are recovered from the + snapshot recorded in _stop_plugin_apps. """ if plugin_ns is not None: self.logger.info(f"Processing restart for plugin namespace '{plugin_ns}'") app_names = self.get_namespace_apps(plugin_ns) + if popped := self.stopped_plugin_apps.pop(plugin_ns, set()): + # Filter out apps whose config is gone, disabled, or not an app anymore + app_names |= { + name for name in popped + if isinstance(cfg := self.app_config.root.get(name), AppConfig) and not cfg.disable + } # fmt: skip deps = self.dependency_manager.app_deps.get_dependents(app_names) update_actions.apps.init |= deps diff --git a/appdaemon/plugin_management.py b/appdaemon/plugin_management.py index 18af89470..bd2ac51cf 100644 --- a/appdaemon/plugin_management.py +++ b/appdaemon/plugin_management.py @@ -228,6 +228,9 @@ class PluginManagement: "active": , "name": }}`` + + The ``active`` flag is purely a runtime failure signal: True from registration, set False only by + ``notify_plugin_stopped``, and set True again by ``notify_plugin_started``. """ required_meta = ["latitude", "longitude", "elevation", "time_zone"] last_plugin_state: dict[str, datetime.datetime] @@ -311,7 +314,7 @@ def __init__(self, ad: "AppDaemon", config: dict[str, PluginConfig]): case {"object": PluginBase(name=str(existing_plugin))}: raise ade.PluginNamespaceError(name, namespace, existing_plugin) - self.plugin_objs[namespace] = {"object": plugin, "active": False, "name": name} + self._register_plugin(name, plugin, namespace) if self.AD.apps_enabled: # Create app entry for the plugin so we can listen_state/event @@ -327,6 +330,26 @@ def __init__(self, ad: "AppDaemon", config: dict[str, PluginConfig]): self.logger.warning(traceback.format_exc()) self.logger.warning("-" * 60) + def _register_plugin(self, name: str, plugin: PluginBase, namespace: str) -> None: + """Records an instantiated plugin in ``plugin_objs`` and seeds its state-refresh timestamp. + + The ``active`` flag tracks plugin failure at runtime only: it starts True and is flipped by + ``notify_plugin_stopped``/``notify_plugin_started``. A plugin that has never connected does + NOT count as inactive — its un-readiness blocks app loading via ``wait_for_plugins``, which + waits on ready events that ``notify_plugin_started`` sets. Never-connected plugin failure + does not call ``notify_plugin_stopped``, so the flag stays True and the guard in + ``app_management.check_app_updates`` stays off at boot. + + The ``last_plugin_state`` entry is seeded so ``time_since_plugin_update`` cannot hit a + KeyError in the window between registration and ``notify_plugin_started`` (a plugin whose + ready event is set but whose start notification hasn't completed is already "active"). + """ + self.plugin_objs[namespace] = {"object": plugin, "active": True, "name": name} + # Seed the refresh timestamp directly instead of via sched.get_now_sync(), which is + # deprecated-datetime based; only the elapsed delta is ever compared, so a naive UTC now + # is equivalent to what refresh_update_time records later + self.last_plugin_state[name] = datetime.datetime.now(tz=datetime.timezone.utc) + @property def plugin_dir(self) -> Path: """Built-in plugin base directory""" @@ -362,10 +385,19 @@ async def stop(self): continue self.logger.debug("Stopping plugin '%s'", name) - if asyncio.iscoroutinefunction(stop_func): - await stop_func() - else: - stop_func() + try: + if asyncio.iscoroutinefunction(stop_func): + await stop_func() + else: + stop_func() + except Exception: + # A misbehaving plugin must not abort the shutdown sequence for the other + # plugins; their callbacks/futures still get cleaned up below + self.logger.warning("-" * 60) + self.logger.warning("Error stopping plugin: %s - continuing shutdown", name) + self.logger.warning("-" * 60) + self.logger.warning(traceback.format_exc()) + self.logger.warning("-" * 60) await self.AD.callbacks.clear_callbacks(name) self.AD.futures.cancel_futures(name) @@ -437,6 +469,10 @@ def get_plugin_from_namespace(self, namespace: str) -> str: else: raise NameError(f"Bad namespace: {namespace}") + def any_plugin_inactive(self) -> bool: + """Whether any configured plugin is currently marked inactive.""" + return any(not plugin_cfg.get("active", True) for plugin_cfg in self.plugin_objs.values()) + async def notify_plugin_stopped(self, name: str, namespace: str): self.plugin_objs[namespace]["active"] = False data = {"event_type": "plugin_stopped", "data": {"name": name}} diff --git a/appdaemon/plugins/hass/hassplugin.py b/appdaemon/plugins/hass/hassplugin.py index d495b4d2f..35542d98b 100644 --- a/appdaemon/plugins/hass/hassplugin.py +++ b/appdaemon/plugins/hass/hassplugin.py @@ -67,10 +67,15 @@ def check_received_event(self, event: dict): class HassPlugin(PluginBase): config: HASSConfig id: int - session: aiohttp.ClientSession - """http connection pool for general use""" - ws: aiohttp.ClientWebSocketResponse - """websocket dedicated for event loop""" + session: aiohttp.ClientSession | None + """http connection pool for general use. + + Only assigned inside :py:meth:`~HassPlugin.websocket_msg_factory`, so it is ``None`` until a + websocket connect succeeds, notably for a plugin whose Home Assistant instance was never + reachable. Methods may rely on it being set whenever a connection has been established. + """ + ws: aiohttp.ClientWebSocketResponse | None + """websocket dedicated for event loop, ``None`` until a websocket connect succeeds""" metadata: dict[str, Any] services: dict[ str, # Domain @@ -108,15 +113,22 @@ def __init__(self, ad: "AppDaemon", name: str, config: HASSConfig): self.startup_conditions = [] self.maintenance_tasks = [] + # Sentinels: ws and session are only assigned by websocket_msg_factory() after a successful + # websocket connect, so stop() must tolerate a plugin that never connected (e.g. Home + # Assistant was unreachable at boot) + self.ws = None + self.session = None + self.service_logger = self.diag.getChild("services") self.logger.info("HASS Plugin initialization complete") async def stop(self): - await self.ws.close() - self.logger.debug("Websocket closed for '%s'", self.name) - - await self.session.close() - self.logger.debug("aiohttp session closed for '%s'", self.name) + if self.ws is not None: + await self.ws.close() + self.logger.debug("Websocket closed for '%s'", self.name) + if self.session is not None: + await self.session.close() + self.logger.debug("aiohttp session closed for '%s'", self.name) def _create_maintenance_task(self, coro: Coroutine, name: str) -> asyncio.Task: task = self.AD.loop.create_task(coro, name=name) diff --git a/tests/functional/test_plugin_guard.py b/tests/functional/test_plugin_guard.py new file mode 100644 index 000000000..a545cf8f9 --- /dev/null +++ b/tests/functional/test_plugin_guard.py @@ -0,0 +1,335 @@ +import pytest +from appdaemon.models.config.plugin import PluginConfig +from appdaemon.models.internal.app_management import ManagedObject, UpdateMode +from appdaemon.plugin_management import PluginBase + +from tests.conftest import ConfiguredAppDaemonFunc + +APP_NAME = "hello_world" +ANOTHER_APP = "another_app" +PLUGIN_NS = "default" +OTHER_PLUGIN_NS = "other" + + +class RegistrationStubPlugin(PluginBase): + """Minimal plugin so the real registration seam can be exercised without any I/O.""" + + async def get_updates(self): + pass + + async def get_complete_state(self): + return {} + + +def plugin_entry(active: bool) -> dict: + return { + "object": object(), + "active": active, + "name": "test", + } + + +def assert_app_running(ad, app_name: str, msg: str) -> None: + match ad.app_management.objects.get(app_name): + case ManagedObject(type="app", running=True): + return + case _: + pytest.fail(msg) + + +@pytest.mark.ci +@pytest.mark.functional +@pytest.mark.asyncio(loop_scope="session") +async def test_inactive_plugin_blocks_normal_restart(configured_appdaemon: ConfiguredAppDaemonFunc) -> None: + """A NORMAL update pass must not restart apps of a plugin marked inactive.""" + app_cfgs = {APP_NAME: {"module": "hello", "class": "HelloWorld"}} + + async with configured_appdaemon(app_cfgs=app_cfgs, loggers=["_app_management"]) as (ad, caplog): + await ad.utility.app_update_event.wait() + assert APP_NAME in ad.app_management.objects, "sanity: app should start initially" + + # Simulate plugin failure like notify_plugin_stopped does + ad.plugins.plugin_objs[PLUGIN_NS] = plugin_entry(active=False) + + # Mimic PLUGIN_FAILED stopping the app + stopped = await ad.app_management.stop_app(APP_NAME, delete=True) + assert stopped, "sanity: app should stop cleanly" + assert APP_NAME not in ad.app_management.objects + + caplog.clear() + + # NORMAL pass from the utility loop while the plugin is down + ad.utility.app_update_event.clear() + await ad.app_management.check_app_updates() + + assert APP_NAME not in ad.app_management.objects, "NORMAL pass should not repopulate init while a plugin is inactive" + assert "Skipping init repopulation" in caplog.text, "guard branch should log skipping init repopulation" + assert not any(r.levelname == "ERROR" for r in caplog.records), "no errors expected while guard is active" + + # PLUGIN_RESTART after reconnection starts the app again. Mirror notify_plugin_started, + # which sets active=True before scheduling PLUGIN_RESTART. + ad.plugins.plugin_objs[PLUGIN_NS]["active"] = True + await ad.app_management.check_app_updates(plugin_ns=PLUGIN_NS, mode=UpdateMode.PLUGIN_RESTART) + assert_app_running(ad, APP_NAME, "app should be running after PLUGIN_RESTART") + + +@pytest.mark.ci +@pytest.mark.functional +@pytest.mark.asyncio(loop_scope="session") +async def test_plugin_restart_starts_apps(configured_appdaemon: ConfiguredAppDaemonFunc) -> None: + """After a plugin reconnects, PLUGIN_RESTART must start apps recovered from the stopped-app snapshot. + + A second plugin entry stays inactive the whole time so the init-repopulation guard remains on, + and the app's ManagedObject was deleted by the failed plugin — only the snapshot can restore it. + """ + app_cfgs = {APP_NAME: {"module": "hello", "class": "HelloWorld"}} + + async with configured_appdaemon(app_cfgs=app_cfgs) as (ad, caplog): + await ad.utility.app_update_event.wait() + assert APP_NAME in ad.app_management.objects, "sanity: app should start initially" + + # The failing plugin and a second, still-inactive plugin + ad.plugins.plugin_objs[PLUGIN_NS] = plugin_entry(active=False) + ad.plugins.plugin_objs[OTHER_PLUGIN_NS] = {"object": None, "active": False, "name": "other"} + + # PLUGIN_FAILED with the plugin inactive exercises _stop_plugin_apps and the snapshot + await ad.app_management.check_app_updates(plugin_ns=PLUGIN_NS, mode=UpdateMode.PLUGIN_FAILED) + assert APP_NAME not in ad.app_management.objects, "sanity: PLUGIN_FAILED should stop the app" + assert ad.app_management.stopped_plugin_apps[PLUGIN_NS] == {APP_NAME}, "sanity: snapshot should be recorded" + + # Simulate reconnection: only the restarted plugin becomes active again, the second + # plugin stays inactive so the guard remains on and the generic repopulation can't fire + ad.plugins.plugin_objs[PLUGIN_NS]["active"] = True + assert ad.plugins.any_plugin_inactive(), "sanity: guard should still be on" + await ad.app_management.check_app_updates(plugin_ns=PLUGIN_NS, mode=UpdateMode.PLUGIN_RESTART) + assert_app_running(ad, APP_NAME, "app should be running after PLUGIN_RESTART despite the inactive second plugin") + + # Snapshot for this namespace is consumed on restart + assert PLUGIN_NS not in ad.app_management.stopped_plugin_apps + + +@pytest.mark.ci +@pytest.mark.functional +@pytest.mark.asyncio(loop_scope="session") +async def test_plugin_restart_isolated_to_namespace(configured_appdaemon: ConfiguredAppDaemonFunc) -> None: + """Restarting one plugin must not start apps snapshotted for a different, still-failed plugin.""" + app_cfgs = { + APP_NAME: {"module": "hello", "class": "HelloWorld"}, + ANOTHER_APP: {"module": "hello", "class": "HelloWorld"}, + } + + async with configured_appdaemon(app_cfgs=app_cfgs) as (ad, caplog): + await ad.utility.app_update_event.wait() + assert APP_NAME in ad.app_management.objects, "sanity: app should start initially" + assert ANOTHER_APP in ad.app_management.objects, "sanity: app should start initially" + + # The other plugin failed earlier and stopped another_app; it's still down + ad.plugins.plugin_objs[OTHER_PLUGIN_NS] = {"object": None, "active": False, "name": "other"} + stopped = await ad.app_management.stop_app(ANOTHER_APP, delete=True) + assert stopped, "sanity: app should stop cleanly" + # Hand-injected because a real PLUGIN_FAILED pass for ns "other" can't be driven here: it + # requires app objects with that namespace, but apps only get a non-default namespace via + # set_namespace in their own initialize, which HelloWorld doesn't do. This simulates what + # notify_plugin_stopped for that plugin would have recorded. + ad.app_management.stopped_plugin_apps[OTHER_PLUGIN_NS] = {ANOTHER_APP} + + # Now the default plugin fails; drive the real stop path + ad.plugins.plugin_objs[PLUGIN_NS] = plugin_entry(active=False) + await ad.app_management.check_app_updates(plugin_ns=PLUGIN_NS, mode=UpdateMode.PLUGIN_FAILED) + assert APP_NAME not in ad.app_management.objects, "sanity: PLUGIN_FAILED should stop the app" + # another_app was already gone, so it must not leak into this namespace's snapshot + assert ad.app_management.stopped_plugin_apps[PLUGIN_NS] == {APP_NAME}, "only directly stopped apps belong to the snapshot" + + # The default plugin reconnects while the other is still inactive + ad.plugins.plugin_objs[PLUGIN_NS]["active"] = True + await ad.app_management.check_app_updates(plugin_ns=PLUGIN_NS, mode=UpdateMode.PLUGIN_RESTART) + + # Its own snapshot is restored and consumed... + assert_app_running(ad, APP_NAME, "the restarted plugin's app should be running again") + assert PLUGIN_NS not in ad.app_management.stopped_plugin_apps + + # ...while the other plugin's snapshotted app stays stopped — no cross-plugin leaks + assert ANOTHER_APP not in ad.app_management.objects, "app of the still-inactive plugin must not be started" + assert ad.app_management.stopped_plugin_apps[OTHER_PLUGIN_NS] == {ANOTHER_APP}, "snapshot of the other plugin must be untouched" + + # Other plugin still down, so the guard keeps NORMAL passes from starting its app + ad.utility.app_update_event.clear() + await ad.app_management.check_app_updates() + assert ANOTHER_APP not in ad.app_management.objects, "NORMAL pass must not start the other plugin's app" + assert ad.app_management.stopped_plugin_apps[OTHER_PLUGIN_NS] == {ANOTHER_APP}, "snapshot of the other plugin must survive the NORMAL pass" + + +@pytest.mark.ci +@pytest.mark.functional +@pytest.mark.asyncio(loop_scope="session") +async def test_plugin_restart_consumes_snapshot(configured_appdaemon: ConfiguredAppDaemonFunc) -> None: + """A successful PLUGIN_RESTART must pop the namespace's snapshot, whatever it contained.""" + app_cfgs = {APP_NAME: {"module": "hello", "class": "HelloWorld"}} + + async with configured_appdaemon(app_cfgs=app_cfgs) as (ad, caplog): + await ad.utility.app_update_event.wait() + assert APP_NAME in ad.app_management.objects, "sanity: app should start initially" + + ad.plugins.plugin_objs[PLUGIN_NS] = plugin_entry(active=False) + await ad.app_management.check_app_updates(plugin_ns=PLUGIN_NS, mode=UpdateMode.PLUGIN_FAILED) + assert ad.app_management.stopped_plugin_apps[PLUGIN_NS] == {APP_NAME}, "sanity: snapshot should be recorded" + + ad.plugins.plugin_objs[PLUGIN_NS]["active"] = True + await ad.app_management.check_app_updates(plugin_ns=PLUGIN_NS, mode=UpdateMode.PLUGIN_RESTART) + + assert_app_running(ad, APP_NAME, "app should be running after PLUGIN_RESTART") + assert PLUGIN_NS not in ad.app_management.stopped_plugin_apps, "snapshot key should be popped after restart" + + +@pytest.mark.ci +@pytest.mark.functional +@pytest.mark.asyncio(loop_scope="session") +async def test_boot_with_registered_plugin_starts_apps(configured_appdaemon: ConfiguredAppDaemonFunc) -> None: + """Apps must start on the initial NORMAL load even though a plugin was just registered. + + Two pins against the active=True registration: + - the registration seam records an entry with "active": True that never counts as inactive + (with the old active=False this assertion fails), + - a boot where plugins are registered but not yet started does not block the initial load: + empty plugin_objs and freshly registered ones alike must let the NORMAL pass start apps. + """ + app_cfgs = {APP_NAME: {"module": "hello", "class": "HelloWorld"}} + + async with configured_appdaemon(app_cfgs=app_cfgs) as (ad, caplog): + # Pin the registration VALUE via the real seam a never-connected plugin goes through. + # (Calling the full registration path requires importable plugin modules and an app + # management entry plus a get_updates task, so the seam method is driven directly with the + # same arguments the registration loop computes.) The plugin config must exist for + # active_plugins/config lookups. The ready event is set to mirror a plugin that connected + # successfully, matching the state the registration loop would see. + cfg = PluginConfig(name="test", type="dummy") + ad.plugins.config["test"] = cfg + plugin = RegistrationStubPlugin(ad, "test", cfg) + plugin.ready_event.set() + ad.plugins._register_plugin("test", plugin, PLUGIN_NS) + + match entry := ad.plugins.plugin_objs[PLUGIN_NS]: + case {"object": PluginBase(), "active": True, "name": "test"}: + pass + case _: + pytest.fail(f"registration must record the plugin as active, got: {entry}") + assert not ad.plugins.any_plugin_inactive(), "freshly registered plugin must not count as inactive" + + # Boot with no plugins registered yet (plugin_objs empty) doesn't block the initial load + # either: drive the same NORMAL pass AppManagement.start() uses and pin the guard-off path. + caplog.clear() + await ad.app_management.check_app_updates() + assert not any( + "Skipping init repopulation" in r.getMessage() for r in caplog.records + ), "guard must be off during an honest boot with no inactive plugins" + assert_app_running(ad, APP_NAME, "app must start on the initial NORMAL pass with a registered plugin") + + +@pytest.mark.ci +@pytest.mark.functional +@pytest.mark.asyncio(loop_scope="session") +async def test_plugin_failed_twice_merges_snapshot(configured_appdaemon: ConfiguredAppDaemonFunc) -> None: + """A second PLUGIN_FAILED pass during the same outage must not drop the recorded snapshot.""" + app_cfgs = {APP_NAME: {"module": "hello", "class": "HelloWorld"}} + + async with configured_appdaemon(app_cfgs=app_cfgs) as (ad, caplog): + await ad.utility.app_update_event.wait() + assert APP_NAME in ad.app_management.objects, "sanity: app should start initially" + + # First PLUGIN_FAILED pass: the app is still running, so it gets stopped and recorded + ad.plugins.plugin_objs[PLUGIN_NS] = plugin_entry(active=False) + await ad.app_management.check_app_updates(plugin_ns=PLUGIN_NS, mode=UpdateMode.PLUGIN_FAILED) + assert APP_NAME not in ad.app_management.objects, "sanity: PLUGIN_FAILED should stop the app" + assert ad.app_management.stopped_plugin_apps[PLUGIN_NS] == {APP_NAME}, "sanity: snapshot should be recorded" + + # Second PLUGIN_FAILED pass: the app is already gone, so get_namespace_apps is empty. The + # snapshot must survive via the setdefault+|= merge, not be overwritten with an empty set. + await ad.app_management.check_app_updates(plugin_ns=PLUGIN_NS, mode=UpdateMode.PLUGIN_FAILED) + assert ad.app_management.stopped_plugin_apps[PLUGIN_NS] == {APP_NAME}, "second pass must preserve the snapshot" + + # PLUGIN_RESTART after reconnection still starts it + ad.plugins.plugin_objs[PLUGIN_NS]["active"] = True + await ad.app_management.check_app_updates(plugin_ns=PLUGIN_NS, mode=UpdateMode.PLUGIN_RESTART) + assert_app_running(ad, APP_NAME, "app should be running after PLUGIN_RESTART") + + +@pytest.mark.ci +@pytest.mark.functional +@pytest.mark.asyncio(loop_scope="session") +async def test_snapshot_filter_disabled_app(configured_appdaemon: ConfiguredAppDaemonFunc) -> None: + """PLUGIN_RESTART must skip snapshotted apps whose config is now disabled.""" + app_cfgs = {APP_NAME: {"module": "hello", "class": "HelloWorld"}} + + async with configured_appdaemon(app_cfgs=app_cfgs) as (ad, caplog): + await ad.utility.app_update_event.wait() + assert APP_NAME in ad.app_management.objects, "sanity: app should start initially" + + # Plugin fails and the app gets stopped and recorded + ad.plugins.plugin_objs[PLUGIN_NS] = plugin_entry(active=False) + await ad.app_management.check_app_updates(plugin_ns=PLUGIN_NS, mode=UpdateMode.PLUGIN_FAILED) + assert ad.app_management.stopped_plugin_apps[PLUGIN_NS] == {APP_NAME}, "sanity: snapshot should be recorded" + + # While the plugin is down, someone disables the app's config + ad.app_management.app_config.root[APP_NAME].disable = True + + # PLUGIN_RESTART must not start the disabled app (pop-time filter) and must not blow up in + # start_sort on app names whose config is no longer active + ad.plugins.plugin_objs[PLUGIN_NS]["active"] = True + await ad.app_management.check_app_updates(plugin_ns=PLUGIN_NS, mode=UpdateMode.PLUGIN_RESTART) + + match ad.app_management.objects.get(APP_NAME): + case ManagedObject(type="app", running=True): + pytest.fail("disabled app must not be started by PLUGIN_RESTART") + case _: + pass + + # Recovery here goes through the init repopulation in check_app_config_files, not the + # snapshot: the snapshot for this namespace was already popped by the PLUGIN_RESTART above, + # and the app's ManagedObject is gone, so the not-in-objects repopulation is what adds it + # back now that the plugin (and thus the guard) is active again + ad.app_management.app_config.root[APP_NAME].disable = False + await ad.app_management.check_app_updates(plugin_ns=PLUGIN_NS, mode=UpdateMode.PLUGIN_RESTART) + assert_app_running(ad, APP_NAME, "re-enabled app should start on the next PLUGIN_RESTART") + + +@pytest.mark.ci +@pytest.mark.functional +@pytest.mark.asyncio(loop_scope="session") +async def test_plugin_restart_merges_namespace_apps(configured_appdaemon: ConfiguredAppDaemonFunc) -> None: + """PLUGIN_RESTART must union apps still present via get_namespace_apps with the snapshot. + + An app re-created while the plugin was down (e.g. created/edited during the outage) has a valid + ManagedObject again, so get_namespace_apps finds it alongside the snapshot apps; the union must + start everything without double-starting or raising. + """ + app_cfgs = {APP_NAME: {"module": "hello", "class": "HelloWorld"}} + + async with configured_appdaemon(app_cfgs=app_cfgs) as (ad, caplog): + await ad.utility.app_update_event.wait() + assert APP_NAME in ad.app_management.objects, "sanity: app should start initially" + + # Plugin fails and the app gets stopped and recorded + ad.plugins.plugin_objs[PLUGIN_NS] = plugin_entry(active=False) + await ad.app_management.check_app_updates(plugin_ns=PLUGIN_NS, mode=UpdateMode.PLUGIN_FAILED) + assert APP_NAME not in ad.app_management.objects, "sanity: PLUGIN_FAILED should stop the app" + assert ad.app_management.stopped_plugin_apps[PLUGIN_NS] == {APP_NAME}, "sanity: snapshot should be recorded" + + # While the plugin is down, the app is re-created out of band (simulating a create/edit + # during the outage): the ManagedObject exists again but the app isn't started + assert await ad.app_management.create_app_object(APP_NAME) is not None, "sanity: app object should re-create" + match ad.app_management.objects.get(APP_NAME): + case ManagedObject(type="app", running=False): + pass + case _: + pytest.fail("sanity: re-created app should exist but not be running") + assert ad.app_management.get_namespace_apps(PLUGIN_NS) == {APP_NAME}, "sanity: namespace apps should contain the re-created app" + + # PLUGIN_RESTART with both sources present: snapshot union with get_namespace_apps + ad.plugins.plugin_objs[PLUGIN_NS]["active"] = True + await ad.app_management.check_app_updates(plugin_ns=PLUGIN_NS, mode=UpdateMode.PLUGIN_RESTART) + + assert_app_running(ad, APP_NAME, "app must be running after the union restart, exactly once") + assert not any( + "Cannot start app" in r.getMessage() for r in caplog.records + ), "the union must not attempt to double-start the app" + assert PLUGIN_NS not in ad.app_management.stopped_plugin_apps, "snapshot should be consumed despite the union" \ No newline at end of file From 16b94b8d62927c866bde8e7120bc8cd45ec71123 Mon Sep 17 00:00:00 2001 From: Peter Szabados Date: Tue, 6 Oct 2026 19:40:16 +0200 Subject: [PATCH 3/5] Clean up comments --- appdaemon/app_management.py | 10 +--------- appdaemon/plugin_management.py | 13 ------------- appdaemon/plugins/hass/hassplugin.py | 10 +--------- 3 files changed, 2 insertions(+), 31 deletions(-) diff --git a/appdaemon/app_management.py b/appdaemon/app_management.py index a3761d90c..bfb1d9550 100644 --- a/appdaemon/app_management.py +++ b/appdaemon/app_management.py @@ -90,8 +90,7 @@ def __init__(self, ad: "AppDaemon"): self.diag = ad.logging.get_diag() self.filter_files = {} self.objects = {} - # Namespace -> apps that were stopped when the plugin failed. Used by _start_plugin_apps - # to restore them, since their ManagedObjects are deleted and no longer carry the namespace + # Namespace -> apps that were stopped when the plugin failed. self.stopped_plugin_apps: dict[str, set[str]] = {} # Add Path for adbase @@ -793,11 +792,6 @@ async def check_app_config_files(self, update_actions: UpdateActions): self.dependency_manager.app_deps.refresh_dep_graph() if self.AD.plugins.any_plugin_inactive(): - # Don't auto-restart apps while a plugin is down - they'll be started - # again when the plugin reconnects (PLUGIN_RESTART) - # Scope: this guard covers only the repopulation of apps that aren't in self.objects. - # Loads initiated by an explicit config-file change (init/reload adds above) are NOT - # guarded — an explicit user action during a plugin outage is honored. self.logger.debug("Skipping init repopulation because a plugin is inactive") else: update_actions.apps.init |= { @@ -1193,8 +1187,6 @@ async def _stop_plugin_apps(self, plugin_ns: str | None, update_actions: UpdateA if plugin_ns is not None: self.logger.info(f"Stopping apps from namespace '{plugin_ns}' because the plugin failed") app_names = self.get_namespace_apps(plugin_ns) - # Snapshot the app names before they're stopped and their ManagedObjects deleted, so - # _start_plugin_apps can restore them when the plugin comes back snapshot = self.stopped_plugin_apps.setdefault(plugin_ns, set()) snapshot |= app_names deps = self.dependency_manager.app_deps.get_dependents(app_names) diff --git a/appdaemon/plugin_management.py b/appdaemon/plugin_management.py index bd2ac51cf..79aacfe04 100644 --- a/appdaemon/plugin_management.py +++ b/appdaemon/plugin_management.py @@ -332,17 +332,6 @@ def __init__(self, ad: "AppDaemon", config: dict[str, PluginConfig]): def _register_plugin(self, name: str, plugin: PluginBase, namespace: str) -> None: """Records an instantiated plugin in ``plugin_objs`` and seeds its state-refresh timestamp. - - The ``active`` flag tracks plugin failure at runtime only: it starts True and is flipped by - ``notify_plugin_stopped``/``notify_plugin_started``. A plugin that has never connected does - NOT count as inactive — its un-readiness blocks app loading via ``wait_for_plugins``, which - waits on ready events that ``notify_plugin_started`` sets. Never-connected plugin failure - does not call ``notify_plugin_stopped``, so the flag stays True and the guard in - ``app_management.check_app_updates`` stays off at boot. - - The ``last_plugin_state`` entry is seeded so ``time_since_plugin_update`` cannot hit a - KeyError in the window between registration and ``notify_plugin_started`` (a plugin whose - ready event is set but whose start notification hasn't completed is already "active"). """ self.plugin_objs[namespace] = {"object": plugin, "active": True, "name": name} # Seed the refresh timestamp directly instead of via sched.get_now_sync(), which is @@ -391,8 +380,6 @@ async def stop(self): else: stop_func() except Exception: - # A misbehaving plugin must not abort the shutdown sequence for the other - # plugins; their callbacks/futures still get cleaned up below self.logger.warning("-" * 60) self.logger.warning("Error stopping plugin: %s - continuing shutdown", name) self.logger.warning("-" * 60) diff --git a/appdaemon/plugins/hass/hassplugin.py b/appdaemon/plugins/hass/hassplugin.py index 35542d98b..8ef3b0cad 100644 --- a/appdaemon/plugins/hass/hassplugin.py +++ b/appdaemon/plugins/hass/hassplugin.py @@ -68,12 +68,7 @@ class HassPlugin(PluginBase): config: HASSConfig id: int session: aiohttp.ClientSession | None - """http connection pool for general use. - - Only assigned inside :py:meth:`~HassPlugin.websocket_msg_factory`, so it is ``None`` until a - websocket connect succeeds, notably for a plugin whose Home Assistant instance was never - reachable. Methods may rely on it being set whenever a connection has been established. - """ + """http connection pool for general use, ``None`` until a websocket connect succeeds.""" ws: aiohttp.ClientWebSocketResponse | None """websocket dedicated for event loop, ``None`` until a websocket connect succeeds""" metadata: dict[str, Any] @@ -113,9 +108,6 @@ def __init__(self, ad: "AppDaemon", name: str, config: HASSConfig): self.startup_conditions = [] self.maintenance_tasks = [] - # Sentinels: ws and session are only assigned by websocket_msg_factory() after a successful - # websocket connect, so stop() must tolerate a plugin that never connected (e.g. Home - # Assistant was unreachable at boot) self.ws = None self.session = None From 44c7cab5aa01289c7d311135ef36e1123bd64a0c Mon Sep 17 00:00:00 2001 From: Peter Szabados Date: Tue, 6 Oct 2026 19:48:58 +0200 Subject: [PATCH 4/5] Add missing newline at end of test file --- tests/functional/test_plugin_guard.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/functional/test_plugin_guard.py b/tests/functional/test_plugin_guard.py index a545cf8f9..5088bb2c8 100644 --- a/tests/functional/test_plugin_guard.py +++ b/tests/functional/test_plugin_guard.py @@ -332,4 +332,4 @@ async def test_plugin_restart_merges_namespace_apps(configured_appdaemon: Config assert not any( "Cannot start app" in r.getMessage() for r in caplog.records ), "the union must not attempt to double-start the app" - assert PLUGIN_NS not in ad.app_management.stopped_plugin_apps, "snapshot should be consumed despite the union" \ No newline at end of file + assert PLUGIN_NS not in ad.app_management.stopped_plugin_apps, "snapshot should be consumed despite the union" From 09c044a562f41fa5e285f410436cfe4455958fe4 Mon Sep 17 00:00:00 2001 From: Peter Szabados Date: Sat, 10 Oct 2026 15:14:31 +0200 Subject: [PATCH 5/5] fix(threads): preserve idle state after slow callbacks --- appdaemon/threads.py | 4 +- tests/functional/test_thread_completion.py | 56 ++++++++++++++++++++++ 2 files changed, 58 insertions(+), 2 deletions(-) create mode 100644 tests/functional/test_thread_completion.py diff --git a/appdaemon/threads.py b/appdaemon/threads.py index f2fbd35f3..01bc1654c 100644 --- a/appdaemon/threads.py +++ b/appdaemon/threads.py @@ -455,9 +455,9 @@ async def update_thread_info(self, thread_id, callback, app, type, uuid, silent) if self.AD.real_time and duration >= self.AD.thread_duration_warning_threshold: thread_name = f"thread.{thread_id}" - callback = await self.get_state(thread_name) + completed_callback = await self.get_state(thread_name) self.logger.warning( - f"Excessive time spent in callback {callback}. " + f"Excessive time spent in callback {completed_callback}. " f"Thread entity: '{thread_name}' - now complete after {format_timedelta(duration)} " f"(limit={format_timedelta(self.AD.thread_duration_warning_threshold)})" ) diff --git a/tests/functional/test_thread_completion.py b/tests/functional/test_thread_completion.py new file mode 100644 index 000000000..0ecf7ed4b --- /dev/null +++ b/tests/functional/test_thread_completion.py @@ -0,0 +1,56 @@ +import asyncio +import logging +from datetime import timedelta + +import pytest +from appdaemon.utils.str import dt_to_str + +from tests.conftest import ConfiguredAppDaemonFunc + + +@pytest.mark.ci +@pytest.mark.functional +@pytest.mark.asyncio(loop_scope="session") +@pytest.mark.parametrize("thread_id", ["thread-0", "async"]) +@pytest.mark.parametrize("duration", [1, 12], ids=["fast", "slow"]) +async def test_completed_callback_returns_to_idle( + configured_appdaemon: ConfiguredAppDaemonFunc, + thread_id: str, + duration: int, +) -> None: + """A slow-callback warning must not leave a completed callback busy.""" + app_name = "hello_world" + callback = "HelloWorld.callback for hello_world" + handle = "test_completion" + app_cfgs = {app_name: {"module": "hello", "class": "HelloWorld"}} + + async with configured_appdaemon(app_cfgs=app_cfgs, loggers=["_threading"]) as (ad, caplog): + await ad.utility.app_update_event.wait() + # Avoid duplicate DEBUG messages replacing the warning in DuplicateFilter. + with caplog.at_level(logging.WARNING, logger=ad.threading.logger.name): + await ad.state.add_entity("admin", f"scheduler_callback.{handle}", "active") + await ad.threading.update_thread_info(thread_id, callback, app_name, "scheduler", handle, False) + now = await ad.sched.get_now() + await ad.state.set_state( + "test", + "admin", + f"thread.{thread_id}", + time_called=dt_to_str(now - timedelta(seconds=duration), ad.tz), + ) + await asyncio.sleep(0) + caplog.clear() + + await ad.threading.update_thread_info(thread_id, "idle", app_name, "scheduler", handle, False) + + assert await ad.state.get_state("test", "admin", f"thread.{thread_id}") == "idle" + assert await ad.state.get_state("test", "admin", f"app.{app_name}") == "idle" + warnings = [ + record.getMessage() + for record in caplog.records + if record.levelno == logging.WARNING and "Excessive time spent in callback" in record.getMessage() + ] + if duration == 12: + assert len(warnings) == 1 + assert callback in warnings[0] + else: + assert not warnings