PR Feedback

backend/app/api/routes/settings.py

@@ -219,6 +219,8 @@ _BOOL_SETTING_KEYS = frozenset(
         "local_login_enabled",
         "preheat_enabled",
         "queue_keep_bed_warm",
+        "obico_enabled",
+        "octoeverywhere_enabled",
     }
 )
 
@@ -236,7 +238,9 @@ _FLOAT_SETTING_KEYS = frozenset(
 
 # String settings limited to a fixed set of values, which a stored "None"
 # would break when the settings response is built.
-_ENUM_SETTING_KEYS = frozenset({"energy_price_source"})
+_ENUM_SETTING_KEYS = frozenset(
+    {"energy_price_source", "obico_sensitivity", "obico_action", "octoeverywhere_confidence", "octoeverywhere_action"}
+)
 
 _INT_SETTING_KEYS = frozenset(
     {
@@ -262,6 +266,7 @@ _INT_SETTING_KEYS = frozenset(
         "queue_max_concurrent_uploads",
         "ambient_drying_sustained_minutes",
         "pause_warning_minutes",
+        "obico_poll_interval",
         "octoeverywhere_poll_interval",
     }
 )
@@ -383,16 +388,9 @@ async def update_settings(
     if update_data.get("octoeverywhere_api_key") is None:
         update_data.pop("octoeverywhere_api_key", None)
 
-    # Only one failure detection provider may control a print at a time.
-    if update_data.get("obico_enabled") and update_data.get("octoeverywhere_enabled"):
-        raise HTTPException(status_code=400, detail="Enable only one AI failure detection provider at a time.")
-    if update_data.get("octoeverywhere_enabled"):
-        update_data["obico_enabled"] = False
-    elif update_data.get("obico_enabled"):
-        update_data["octoeverywhere_enabled"] = False
-
-    # An explicit null for a boolean or numeric setting has no meaning -- these
-    # are not clearable -- and would be stored as the literal "None".
+    # These typed settings are not clearable: null would be stored as "None".
+    # Check before normalizing provider flags so enabling one cannot hide a
+    # null submitted for the other.
     null_keys = sorted(
         key
         for key, value in update_data.items()
@@ -401,6 +399,14 @@ async def update_settings(
     if null_keys:
         raise HTTPException(status_code=422, detail=f"These settings cannot be null: {', '.join(null_keys)}")
 
+    # Only one failure detection provider may control a print at a time.
+    if update_data.get("obico_enabled") and update_data.get("octoeverywhere_enabled"):
+        raise HTTPException(status_code=400, detail="Enable only one AI failure detection provider at a time.")
+    if update_data.get("octoeverywhere_enabled"):
+        update_data["obico_enabled"] = False
+    elif update_data.get("obico_enabled"):
+        update_data["octoeverywhere_enabled"] = False
+
     # Settings that decide who can sign in are admin-only, like the user routes.
     # Only a real change counts: the settings page sends every field on each
     # save, so an unchanged value from a non-admin must pass.

backend/app/schemas/settings.py

@@ -894,7 +894,7 @@ class AppSettingsUpdate(BaseModel):
     octoeverywhere_api_key: str | None = None
     # Unset fields default to None; the settings route rejects an explicit null
     # for numeric settings before it can be stored.
-    octoeverywhere_poll_interval: int | None = Field(default=None, ge=5, le=30, strict=True)
+    octoeverywhere_poll_interval: int | None = Field(default=None, ge=5, le=30)
     octoeverywhere_confidence: str | None = None
     octoeverywhere_action: str | None = None
     octoeverywhere_enabled_printers: str | None = None

backend/app/services/obico_actions.py

@@ -21,6 +21,7 @@ async def execute_action(
     frame: bytes | None = None,
     *,
     print_quality: int | None = None,
+    printer_name: str | None = None,
 ) -> None:
     """Run the configured action for a detected print failure.
 
@@ -28,7 +29,10 @@ async def execute_action(
     frame: the camera snapshot the ML model flagged, if any — attached to
         the notification so the recipient can see what triggered it.
     """
-    printer_name = await _get_printer_name(printer_id)
+    # OctoEverywhere resolves the name before committing an action so stale
+    # checks can still be cancelled before any hardware control begins.
+    if printer_name is None:
+        printer_name = await _get_printer_name(printer_id)
 
     if action in ("pause", "pause_and_off"):
         _pause_print(printer_id)

backend/app/services/obico_detection.py

@@ -150,6 +150,7 @@ class ObicoDetectionService:
     async def _load_settings(self) -> dict:
         keys = [
             "obico_enabled",
+            "octoeverywhere_enabled",
             "obico_ml_url",
             "bambuddy_internal_url",
             "obico_ml_token",
@@ -173,7 +174,10 @@ class ObicoDetectionService:
             enabled_printers = None  # None = all printers
 
         return {
-            "enabled": rows.get("obico_enabled", "false").lower() == "true",
+            # Restores can bypass PUT /settings's provider selection. A
+            # conflicting configuration must never start both detectors.
+            "enabled": rows.get("obico_enabled", "false").lower() == "true"
+            and rows.get("octoeverywhere_enabled", "false").lower() != "true",
             "ml_url": (rows.get("obico_ml_url") or "").rstrip("/"),
             "ml_token": (rows.get("obico_ml_token") or "").strip(),
             "sensitivity": rows.get("obico_sensitivity", "medium"),

backend/app/services/octoeverywhere_actions.py

@@ -4,7 +4,15 @@ from backend.app.services.obico_actions import execute_action as execute_detecti
 
 
 async def execute_action(
-    printer_id: int, action: str, task_name: str, print_quality: int, frame: bytes | None = None
+    printer_id: int,
+    action: str,
+    task_name: str,
+    print_quality: int,
+    frame: bytes | None = None,
+    *,
+    printer_name: str | None = None,
 ) -> None:
     """Dispatch a suggested action without treating print quality as confidence."""
-    await execute_detection_action(printer_id, action, task_name, None, frame, print_quality=print_quality)
+    await execute_detection_action(
+        printer_id, action, task_name, None, frame, print_quality=print_quality, printer_name=printer_name
+    )

backend/app/services/octoeverywhere_detection.py

@@ -199,7 +199,9 @@ class OctoEverywhereDetectionService:
 
     def __init__(self):
         self._task: asyncio.Task | None = None
+        self._running = False
         self._checks: dict[int, asyncio.Task] = {}
+        self._actions: set[asyncio.Task] = set()
         self._states: dict[int, _PrintState] = {}
         self._configuration: tuple[str, str] | None = None
         self._settings: dict | None = None
@@ -240,12 +242,14 @@ class OctoEverywhereDetectionService:
         self._refresh_last_error()
 
     async def start(self):
+        self._running = True
         if self._task is not None and not self._task.done():
             return
         logger.info("Starting OctoEverywhere detection service")
         self._task = asyncio.create_task(self._loop())
 
     def stop(self):
+        self._running = False
         if self._task:
             self._task.cancel()
             self._task = None
@@ -253,6 +257,9 @@ class OctoEverywhereDetectionService:
         for task in self._checks.values():
             task.cancel()
         self._checks.clear()
+        for task in self._actions:
+            task.cancel()
+        self._actions.clear()
         self._states.clear()
         self._generation += 1
         self._configuration = None
@@ -286,24 +293,27 @@ class OctoEverywhereDetectionService:
         self._settings = None
         generation = self._generation
         task = self._task
-        restart = task is not None and not task.done()
         self._task = None
-        if task is not None:
-            task.cancel()
-            await asyncio.gather(task, return_exceptions=True)
-        # Printer checks run independently of the loop, so cancel and drain
-        # them before applying settings or restarting the scheduler.
-        checks = [task for task in self._checks.values() if task is not asyncio.current_task()]
-        for task in checks:
-            task.cancel()
-        if checks:
-            await asyncio.gather(*checks, return_exceptions=True)
-        settings = await self._load_settings()
-        if self._generation == generation:
-            self._cache_settings(settings)
-            self._apply_settings(settings)
-        if restart and self._task is None:
-            await self.start()
+        try:
+            if task is not None:
+                task.cancel()
+                await asyncio.gather(task, return_exceptions=True)
+            # Cancel stale inspections, but let already committed actions finish.
+            checks = [task for task in self._checks.values() if task is not asyncio.current_task()]
+            for task in checks:
+                task.cancel()
+            if checks:
+                await asyncio.gather(*checks, return_exceptions=True)
+            settings = await self._load_settings()
+            if self._generation == generation:
+                self._cache_settings(settings)
+                self._apply_settings(settings)
+        finally:
+            # A failed/cancelled refresh must not permanently stop detection.
+            # Lifecycle intent is separate from settings generations so stop()
+            # during this refresh cannot accidentally resurrect the scheduler.
+            if self._running and self._task is None:
+                await self.start()
 
     def _cache_settings(self, settings: dict):
         self._settings = settings
@@ -346,6 +356,7 @@ class OctoEverywhereDetectionService:
 
     async def _load_settings(self) -> dict:
         keys = [
+            "obico_enabled",
             "octoeverywhere_enabled",
             "octoeverywhere_api_key",
             "octoeverywhere_confidence",
@@ -378,7 +389,10 @@ class OctoEverywhereDetectionService:
             poll_interval = DEFAULT_POLL_INTERVAL
 
         return {
-            "enabled": rows.get("octoeverywhere_enabled", "false").lower() == "true",
+            # Restores can bypass PUT /settings and leave both flags enabled.
+            # Refuse to act while the other provider is enabled as well.
+            "enabled": rows.get("octoeverywhere_enabled", "false").lower() == "true"
+            and rows.get("obico_enabled", "false").lower() != "true",
             "api_key": (rows.get("octoeverywhere_api_key") or "").strip(),
             "confidence": rows.get("octoeverywhere_confidence", "medium"),
             "action": rows.get("octoeverywhere_action", "notify"),
@@ -752,16 +766,51 @@ class OctoEverywhereDetectionService:
             # A pause or power cut always notifies, even after a warning: the
             # user needs to know the printer stopped. Notify-only would just
             # repeat the warning, so it stays silent.
-            if action != "notify" or not state.warning_fired:
-                await self._dispatch_action(printer_id, action, task_name, print_quality, frame)
-            state.action_fired = True
-            state.warning_fired = True
+            if action == "notify" and state.warning_fired:
+                state.action_fired = True
+                return
         elif warning and not state.warning_fired:
-            await self._dispatch_action(printer_id, "notify", task_name, print_quality, frame)
-            state.warning_fired = True
+            action = "notify"
+        else:
+            return
+
+        # Preparation is still cancellable: a print/settings change before any
+        # side effect must prevent stale hardware control.
+        printer_name = await self._prepare_action(printer_id)
+        if not self._is_current(printer_id, state, status, generation):
+            return
+        # Once committed, a pause/disconnect or autosave must neither interrupt
+        # the remaining plug-off/notification nor make this print act twice.
+        state.action_fired = state.action_fired or pause
+        state.warning_fired = True
+        action_task = asyncio.create_task(
+            self._dispatch_action(printer_id, action, task_name, print_quality, frame, printer_name=printer_name)
+        )
+        self._actions.add(action_task)
+        action_task.add_done_callback(self._action_done)
+        await asyncio.shield(action_task)
+
+    async def _prepare_action(self, printer_id: int) -> str:
+        from backend.app.services.obico_actions import _get_printer_name
+
+        return await _get_printer_name(printer_id)
+
+    def _action_done(self, task: asyncio.Task):
+        self._actions.discard(task)
+        if not task.cancelled():
+            error = task.exception()
+            if error is not None:
+                logger.error("OctoEverywhere action task failed: %s", type(error).__name__)
 
     async def _dispatch_action(
-        self, printer_id: int, action: str, task_name: str, print_quality: int, frame: bytes | None = None
+        self,
+        printer_id: int,
+        action: str,
+        task_name: str,
+        print_quality: int,
+        frame: bytes | None = None,
+        *,
+        printer_name: str | None = None,
     ):
         from backend.app.services.octoeverywhere_actions import execute_action
 
@@ -769,7 +818,7 @@ class OctoEverywhereDetectionService:
             "OctoEverywhere: print issue on printer %s (quality=%s) — action=%s", printer_id, print_quality, action
         )
         try:
-            await execute_action(printer_id, action, task_name, print_quality, frame)
+            await execute_action(printer_id, action, task_name, print_quality, frame, printer_name=printer_name)
         except Exception as exc:
             self._set_last_error("OctoEverywhere could not execute the configured failure action.")
             logger.error("OctoEverywhere action dispatch failed: %s", type(exc).__name__)

backend/tests/integration/test_octoeverywhere_api.py

@@ -90,9 +90,11 @@ async def test_write_only_key_preserved_replaced_and_removed(async_client):
 
 
 @pytest.mark.integration
-async def test_fixed_interval_round_trip(async_client):
+@pytest.mark.parametrize("as_text", [False, True])
+async def test_fixed_interval_round_trip(async_client, as_text):
     for interval in (5, 30, 20):
-        response = await async_client.put("/api/v1/settings/", json={"octoeverywhere_poll_interval": interval})
+        value = str(interval) if as_text else interval
+        response = await async_client.put("/api/v1/settings/", json={"octoeverywhere_poll_interval": value})
         assert response.status_code == 200
         assert response.json()["octoeverywhere_poll_interval"] == interval
         settings = (await async_client.get("/api/v1/settings/")).json()
@@ -171,7 +173,10 @@ async def test_rejects_two_enabled_providers(async_client):
         {"octoeverywhere_poll_interval": 5.5},
         {"octoeverywhere_poll_interval": True},
         {"octoeverywhere_poll_interval": None},
-        {"octoeverywhere_poll_interval": "20"},
+        {"octoeverywhere_poll_interval": "4"},
+        {"octoeverywhere_poll_interval": "31"},
+        {"octoeverywhere_poll_interval": "5.5"},
+        {"octoeverywhere_poll_interval": "invalid"},
     ],
 )
 async def test_invalid_settings_rejected(async_client, payload):

backend/tests/integration/test_settings_api.py

@@ -91,9 +91,24 @@ class TestSettingsAPI:
 
     @pytest.mark.asyncio
     @pytest.mark.integration
-    @pytest.mark.parametrize("key", ["ambient_drying_sustained_minutes", "default_filament_cost", "auto_archive"])
+    @pytest.mark.parametrize(
+        "key",
+        [
+            "ambient_drying_sustained_minutes",
+            "default_filament_cost",
+            "auto_archive",
+            "obico_enabled",
+            "obico_sensitivity",
+            "obico_action",
+            "obico_poll_interval",
+            "octoeverywhere_enabled",
+            "octoeverywhere_confidence",
+            "octoeverywhere_action",
+            "octoeverywhere_poll_interval",
+        ],
+    )
     async def test_null_for_a_typed_setting_is_refused_and_not_stored(self, async_client: AsyncClient, key):
-        """A null for a boolean or numeric setting would be stored as the literal
+        """A null for a typed setting would be stored as the literal
         "None". The whole request is refused, so a valid field sent alongside it
         is not half-applied either."""
         before = (await async_client.get("/api/v1/settings/")).json()
@@ -103,10 +118,35 @@ class TestSettingsAPI:
 
         assert response.status_code == 422
         assert key in response.json()["detail"]
-        after = (await async_client.get("/api/v1/settings/")).json()
+        response = await async_client.get("/api/v1/settings/")
+        assert response.status_code == 200
+        after = response.json()
         assert after[key] == before[key]
         assert after["currency"] == before["currency"]
 
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    @pytest.mark.parametrize(
+        "enabled_key, null_key",
+        [("obico_enabled", "octoeverywhere_enabled"), ("octoeverywhere_enabled", "obico_enabled")],
+    )
+    async def test_enabling_provider_does_not_hide_null_other_provider(
+        self, async_client: AsyncClient, enabled_key, null_key
+    ):
+        before = (await async_client.get("/api/v1/settings/")).json()
+
+        response = await async_client.put(
+            "/api/v1/settings/", json={enabled_key: True, null_key: None, "currency": "EUR"}
+        )
+
+        assert response.status_code == 422
+        assert null_key in response.json()["detail"]
+        response = await async_client.get("/api/v1/settings/")
+        assert response.status_code == 200
+        after = response.json()
+        for key in (enabled_key, null_key, "currency"):
+            assert after[key] == before[key]
+
     @pytest.mark.asyncio
     @pytest.mark.integration
     async def test_a_set_temp_alarm_reads_back_as_a_float(self, async_client: AsyncClient, db_session):

backend/tests/unit/services/test_octoeverywhere_actions.py

@@ -13,7 +13,21 @@ from backend.app.services.octoeverywhere_actions import execute_action
 async def test_dispatches_action_with_quality_without_fake_confidence(action, frame):
     with patch("backend.app.services.octoeverywhere_actions.execute_detection_action", new_callable=AsyncMock) as run:
         await execute_action(1, action, "benchy", 2, frame)
-    run.assert_awaited_once_with(1, action, "benchy", None, frame, print_quality=2)
+    run.assert_awaited_once_with(1, action, "benchy", None, frame, print_quality=2, printer_name=None)
+
+
+async def test_prepared_action_reuses_printer_name_without_another_lookup():
+    with (
+        patch("backend.app.services.obico_actions._get_printer_name", new_callable=AsyncMock) as lookup,
+        patch("backend.app.services.obico_actions._pause_print") as pause,
+        patch("backend.app.services.obico_actions._turn_off_linked_plugs", new_callable=AsyncMock) as off,
+        patch("backend.app.services.obico_actions._notify", new_callable=AsyncMock) as notify,
+    ):
+        await execute_action(1, "pause_and_off", "benchy", 2, b"frame", printer_name="Prepared printer")
+    lookup.assert_not_awaited()
+    pause.assert_called_once_with(1)
+    off.assert_awaited_once_with(1)
+    notify.assert_awaited_once_with(1, "Prepared printer", "benchy", None, "pause_and_off", b"frame", print_quality=2)
 
 
 async def test_pause_and_power_off_also_notify():

backend/tests/unit/test_failure_detection_provider_exclusivity.py

@@ -0,0 +1,107 @@
+"""Provider selection stays exclusive even when a backup bypasses the settings route."""
+
+import asyncio
+from contextlib import asynccontextmanager
+from types import SimpleNamespace
+from unittest.mock import AsyncMock, patch
+
+import pytest
+
+from backend.app.models.settings import Settings
+from backend.app.services.github_restore import GitHubRestoreService, _CategoryTally
+from backend.app.services.obico_detection import ObicoDetectionService
+from backend.app.services.octoeverywhere_detection import OctoEverywhereDetectionService
+
+
+@pytest.fixture
+def detectors(db_session):
+    @asynccontextmanager
+    async def session():
+        yield db_session
+
+    obico = ObicoDetectionService()
+    octoeverywhere = OctoEverywhereDetectionService()
+    with (
+        patch("backend.app.services.obico_detection.async_session", session),
+        patch("backend.app.services.octoeverywhere_detection.async_session", session),
+    ):
+        yield obico, octoeverywhere
+    obico.stop()
+    octoeverywhere.stop()
+
+
+@pytest.mark.parametrize(
+    ("obico_flag", "octoeverywhere_flag", "expected"),
+    [
+        ("false", "false", (False, False)),
+        ("true", "false", (True, False)),
+        ("false", "true", (False, True)),
+        ("true", "true", (False, False)),
+        ("true", None, (True, False)),
+        (None, "true", (False, True)),
+        (None, None, (False, False)),
+    ],
+)
+async def test_provider_selection_from_saved_flags(db_session, detectors, obico_flag, octoeverywhere_flag, expected):
+    for key, value in (("obico_enabled", obico_flag), ("octoeverywhere_enabled", octoeverywhere_flag)):
+        if value is not None:
+            db_session.add(Settings(key=key, value=value))
+    await db_session.commit()
+
+    loaded = [await detector._load_settings() for detector in detectors]
+    assert tuple(settings["enabled"] for settings in loaded) == expected
+
+
+async def test_legacy_restore_never_starts_both_providers_even_with_cached_settings(db_session, detectors):
+    obico, octoeverywhere = detectors
+    db_session.add_all(
+        Settings(key=key, value=value)
+        for key, value in {
+            "obico_enabled": "false",
+            "obico_ml_url": "http://obico.local:3333",
+            "octoeverywhere_enabled": "true",
+            "octoeverywhere_api_key": "test-key",
+        }.items()
+    )
+    await db_session.commit()
+    octoeverywhere._cache_settings(await octoeverywhere._load_settings())
+    assert octoeverywhere._settings["enabled"] is True
+
+    tally, written = _CategoryTally(), set()
+    await GitHubRestoreService()._restore_settings(
+        db_session, {"settings": {"obico_enabled": "true"}}, overwrite=True, tally=tally, keys_written=written
+    )
+    await db_session.commit()
+    assert written == {"obico_enabled"}
+
+    # Stop after one scheduler tick, retaining normal event-loop yields used by
+    # the database driver and the independently scheduled printer check.
+    original_sleep = asyncio.sleep
+
+    async def one_tick(delay):
+        if delay > 0:
+            raise asyncio.CancelledError
+        await original_sleep(0)
+
+    obico._check_printer = AsyncMock()
+    octoeverywhere._check_printer = AsyncMock()
+    with (
+        patch("backend.app.services.printer_manager.printer_manager") as manager,
+        patch("asyncio.sleep", one_tick),
+    ):
+        manager.get_all_statuses.return_value = {1: SimpleNamespace(state="RUNNING", task_name="test-print")}
+        manager.is_connected.return_value = True
+        await obico._loop()
+        await octoeverywhere._loop()
+        await asyncio.gather(*octoeverywhere._checks.values())
+        obico._check_printer.assert_not_awaited()
+        octoeverywhere._check_printer.assert_awaited_once()
+
+        # At its next normal settings refresh, OctoEverywhere also sees the
+        # conflicting selection and stops inspecting, without a process restart.
+        octoeverywhere._settings_refresh_at = 0
+        await octoeverywhere._loop()
+        await obico._loop()
+        assert octoeverywhere._settings["enabled"] is False
+        obico._check_printer.assert_not_awaited()
+        octoeverywhere._check_printer.assert_awaited_once()

backend/tests/unit/test_github_restore_validation.py

@@ -8,6 +8,7 @@ unknown) is not restored, and neither is a user's email code sign-in flag.
 import pytest
 from sqlalchemy import select
 
+from backend.app.api.routes.settings import _build_settings_response
 from backend.app.models.settings import Settings
 from backend.app.schemas.github_backup import RestoreCategory
 from backend.app.services.github_restore import _ADDRESS_KEYS, SETTINGS_PATH, GitHubRestoreService, _CategoryTally
@@ -87,6 +88,30 @@ async def test_a_value_the_editor_accepts_is_restored(db_session, key, value):
     assert _note(tally, "settingsUnknownSkipped") is None
 
 
+@pytest.mark.asyncio
+@pytest.mark.parametrize("interval", ["5", "20", "30"])
+async def test_octoeverywhere_interval_restores_from_backup_text(db_session, interval):
+    key = "octoeverywhere_poll_interval"
+    tally, written, stored = await _restore(db_session, {key: interval}, local={key: "10"})
+
+    assert stored[key] == interval
+    assert written == {key}
+    assert _note(tally, "settingsInvalidSkipped") is None
+    response = await _build_settings_response(db_session)
+    assert response.octoeverywhere_poll_interval == int(interval)
+
+
+@pytest.mark.asyncio
+@pytest.mark.parametrize("interval", ["4", "31", "5.5", "invalid"])
+async def test_invalid_octoeverywhere_interval_is_not_restored(db_session, interval):
+    key = "octoeverywhere_poll_interval"
+    tally, written, stored = await _restore(db_session, {key: interval}, local={key: "20"})
+
+    assert stored[key] == "20"
+    assert written == set()
+    assert _note(tally, "settingsInvalidSkipped") is not None
+
+
 @pytest.mark.asyncio
 @pytest.mark.parametrize(
     "key",

backend/tests/unit/test_octoeverywhere_detection.py

@@ -92,6 +92,7 @@ async def poll_once(detector, configuration):
 def detector():
     service = OctoEverywhereDetectionService()
     service._capture_frame = AsyncMock(return_value=FAKE_JPEG)
+    service._prepare_action = AsyncMock(return_value="Printer")
     service._dispatch_action = AsyncMock()
     return service
 
@@ -524,7 +525,9 @@ class TestProcessing:
             httpx.Response(200, json=process_payload(PauseSuggested=True, PrintQuality=2)),
         ]
         await detector._check_printer(1, printer_status(), settings(action="pause"))
-        detector._dispatch_action.assert_awaited_once_with(1, "pause", "test-print", 2, FAKE_JPEG)
+        detector._dispatch_action.assert_awaited_once_with(
+            1, "pause", "test-print", 2, FAKE_JPEG, printer_name="Printer"
+        )
 
     @pytest.mark.parametrize("action", ["pause", "pause_and_off"])
     async def test_pause_after_warning_sends_its_own_notification(self, detector, http_client, clock, action):
@@ -1004,7 +1007,9 @@ class TestFasterInspectionTiming:
         assert state.frame_count == 2
         assert state.error is None
         assert state.verdict == "warning"
-        detector._dispatch_action.assert_awaited_once_with(1, "notify", "test-print", 4, FAKE_JPEG)
+        detector._dispatch_action.assert_awaited_once_with(
+            1, "notify", "test-print", 4, FAKE_JPEG, printer_name="Printer"
+        )
         assert "private-recommendation" not in "\n".join(
             record.message for record in caplog.records if record.name == MODULE
         )
@@ -2462,6 +2467,81 @@ class TestLifecycle:
         detector._poll_once.assert_awaited_once_with(updated)
         assert detector._settings is updated
 
+    @pytest.mark.parametrize("failure", ["error", "cancel"])
+    async def test_failed_refresh_restarts_scheduler_and_reloads_settings(self, detector, failure):
+        polled = asyncio.Event()
+        loading = asyncio.Event()
+        detector._load_settings = AsyncMock(return_value=settings())
+        detector._poll_once = AsyncMock(side_effect=lambda configuration: polled.set())
+        await detector.start()
+        await asyncio.wait_for(polled.wait(), timeout=1)
+        polled.clear()
+        updated = settings(action="pause")
+
+        async def load_settings():
+            if detector._load_settings.await_count == 1:
+                if failure == "error":
+                    raise RuntimeError("database temporarily unavailable")
+                loading.set()
+                await asyncio.Event().wait()
+            return updated
+
+        detector._load_settings = AsyncMock(side_effect=load_settings)
+        refresh = asyncio.create_task(detector.refresh_settings())
+        try:
+            if failure == "cancel":
+                await asyncio.wait_for(loading.wait(), timeout=1)
+                refresh.cancel()
+            with pytest.raises(RuntimeError if failure == "error" else asyncio.CancelledError):
+                await refresh
+            await asyncio.wait_for(polled.wait(), timeout=1)
+            assert detector.get_status()["is_running"] is True
+            assert detector._settings is updated
+            assert detector._load_settings.await_count == 2
+        finally:
+            task = detector._task
+            detector.stop()
+            await asyncio.gather(task, return_exceptions=True)
+
+    @pytest.mark.parametrize("failure", ["none", "error", "cancel"])
+    async def test_stop_during_refresh_never_restarts_scheduler(self, detector, failure):
+        polled = asyncio.Event()
+        loading = asyncio.Event()
+        release = asyncio.Event()
+        detector._load_settings = AsyncMock(return_value=settings())
+        detector._poll_once = AsyncMock(side_effect=lambda configuration: polled.set())
+        await detector.start()
+        await asyncio.wait_for(polled.wait(), timeout=1)
+
+        async def load_settings():
+            loading.set()
+            await release.wait()
+            if failure == "error":
+                raise RuntimeError("database temporarily unavailable")
+            return settings()
+
+        detector._load_settings = AsyncMock(side_effect=load_settings)
+        refresh = asyncio.create_task(detector.refresh_settings())
+        await asyncio.wait_for(loading.wait(), timeout=1)
+        detector.stop()
+        if failure == "cancel":
+            refresh.cancel()
+        release.set()
+        results = await asyncio.gather(refresh, return_exceptions=True)
+        if failure == "error":
+            assert isinstance(results[0], RuntimeError)
+        elif failure == "cancel":
+            assert isinstance(results[0], asyncio.CancelledError)
+        assert detector._task is None
+        assert detector._settings is None
+        assert detector.get_status()["is_running"] is False
+
+    async def test_refresh_of_stopped_service_does_not_start_scheduler(self, detector):
+        detector._load_settings = AsyncMock(return_value=settings())
+        await detector.refresh_settings()
+        assert detector._task is None
+        assert detector.get_status()["is_running"] is False
+
     async def test_concurrent_refresh_keeps_the_latest_settings_in_cache(self, detector, clock):
         loading = asyncio.Event()
         release = asyncio.Event()
@@ -2511,6 +2591,186 @@ class TestLifecycle:
         detector.stop()
         await asyncio.gather(task, return_exceptions=True)
 
+    async def test_overlapping_refreshes_keep_one_scheduler_and_latest_settings(self, detector):
+        loading = asyncio.Event()
+        release = asyncio.Event()
+        updated = settings(action="pause")
+
+        async def loop():
+            await asyncio.Event().wait()
+
+        async def load_settings():
+            if detector._load_settings.await_count == 1:
+                loading.set()
+                await release.wait()
+                return settings()
+            return updated
+
+        detector._loop = AsyncMock(side_effect=loop)
+        detector._load_settings = AsyncMock(side_effect=load_settings)
+        await detector.start()
+        await asyncio.sleep(0)
+        refresh = asyncio.create_task(detector.refresh_settings())
+        try:
+            await asyncio.wait_for(loading.wait(), timeout=1)
+            await detector.refresh_settings()
+            replacement = detector._task
+            release.set()
+            await asyncio.wait_for(refresh, timeout=1)
+            assert detector._task is replacement
+            assert detector._settings is updated
+            assert detector._loop.await_count == 2
+        finally:
+            release.set()
+            task = detector._task
+            detector.stop()
+            await asyncio.gather(refresh, task, return_exceptions=True)
+
+    @pytest.mark.parametrize(
+        "action, blocked_stage, change",
+        [
+            ("notify", "notification", "refresh"),
+            ("pause", "notification", "pause"),
+            ("pause_and_off", "plug", "pause"),
+            ("pause_and_off", "plug", "disconnect"),
+            ("pause_and_off", "notification", "disconnect"),
+            ("pause_and_off", "notification", "refresh"),
+        ],
+    )
+    async def test_committed_action_finishes_once_despite_check_cancellation(
+        self, detector, http_client, manager, clock, action, blocked_stage, change
+    ):
+        blocked = asyncio.Event()
+        release = asyncio.Event()
+        configuration = settings(action=action)
+        detector._dispatch_action = OctoEverywhereDetectionService._dispatch_action.__get__(detector)
+        verdict = {"WarningSuggested": True} if action == "notify" else {"PauseSuggested": True}
+        http_client.post.side_effect = [
+            httpx.Response(200, json=context_payload()),
+            httpx.Response(200, json=process_payload(**verdict)),
+            httpx.Response(200, json=process_payload(**verdict)),
+        ]
+
+        async def wait_at_stage(stage):
+            if stage == blocked_stage:
+                blocked.set()
+                await release.wait()
+
+        with (
+            patch("backend.app.services.obico_actions._get_printer_name", new_callable=AsyncMock) as lookup,
+            patch("backend.app.services.obico_actions._pause_print") as pause,
+            patch(
+                "backend.app.services.obico_actions._turn_off_linked_plugs",
+                new_callable=AsyncMock,
+            ) as off,
+            patch("backend.app.services.obico_actions._notify", new_callable=AsyncMock) as notify,
+        ):
+
+            async def turn_off(*args):
+                await wait_at_stage("plug")
+
+            async def send_notification(*args, **kwargs):
+                await wait_at_stage("notification")
+
+            off.side_effect = turn_off
+            notify.side_effect = send_notification
+            poll = asyncio.create_task(poll_once(detector, configuration))
+            try:
+                await asyncio.wait_for(blocked.wait(), timeout=1)
+                committed = tuple(detector._actions)
+                assert len(committed) == 1
+                assert detector._states[1].warning_fired is True
+                assert detector._states[1].action_fired is (action != "notify")
+                if change == "refresh":
+                    detector._load_settings = AsyncMock(return_value=configuration)
+                    await detector.refresh_settings()
+                else:
+                    if change == "pause":
+                        manager.get_all_statuses.return_value[1].state = "PAUSE"
+                    else:
+                        manager.is_connected.return_value = False
+                    await detector._poll_once(configuration)
+                await asyncio.wait_for(poll, timeout=1)
+                assert not committed[0].done()
+
+                # Resuming/reconnecting the same print, or the next autosaved
+                # warning assessment, must not enqueue the action again.
+                manager.get_all_statuses.return_value[1].state = "RUNNING"
+                manager.is_connected.return_value = True
+                clock.return_value = 140
+                await poll_once(detector, configuration)
+                assert tuple(detector._actions) == committed
+                release.set()
+                await asyncio.wait_for(asyncio.gather(*committed), timeout=1)
+                assert detector._actions == set()
+                lookup.assert_not_awaited()
+                assert pause.call_count == (action != "notify")
+                assert off.await_count == (action == "pause_and_off")
+                notify.assert_awaited_once()
+            finally:
+                release.set()
+                remaining_actions = tuple(detector._actions)
+                detector.stop()
+                await asyncio.gather(poll, *remaining_actions, return_exceptions=True)
+
+    @pytest.mark.parametrize("change", ["pause", "new_print", "settings"])
+    async def test_action_preparation_rechecks_print_before_committing(self, detector, http_client, change):
+        status = printer_status()
+
+        async def prepare(printer_id):
+            if change == "pause":
+                status.state = "PAUSE"
+            elif change == "new_print":
+                status.subtask_id = "print-2"
+            else:
+                detector._generation += 1
+            return "Printer"
+
+        detector._prepare_action.side_effect = prepare
+        http_client.post.side_effect = [
+            httpx.Response(200, json=context_payload()),
+            httpx.Response(200, json=process_payload(PauseSuggested=True)),
+        ]
+        await detector._check_printer(1, status, settings(action="pause_and_off"))
+        detector._dispatch_action.assert_not_awaited()
+        assert detector._states[1].warning_fired is False
+        assert detector._states[1].action_fired is False
+        assert detector._actions == set()
+
+    async def test_stop_cancels_owned_action_and_cleans_up(self, detector, http_client, manager):
+        started = asyncio.Event()
+
+        async def dispatch(*args, **kwargs):
+            started.set()
+            await asyncio.Event().wait()
+
+        detector._dispatch_action.side_effect = dispatch
+        http_client.post.side_effect = [
+            httpx.Response(200, json=context_payload()),
+            httpx.Response(200, json=process_payload(PauseSuggested=True)),
+        ]
+        poll = asyncio.create_task(poll_once(detector, settings(action="pause")))
+        await asyncio.wait_for(started.wait(), timeout=1)
+        actions = tuple(detector._actions)
+        detector.stop()
+        await asyncio.wait_for(asyncio.gather(poll, *actions, return_exceptions=True), timeout=1)
+        assert actions[0].cancelled()
+        assert detector._actions == set()
+
+    async def test_action_failure_is_reported_and_owned_task_is_removed(self, detector, http_client, manager):
+        detector._dispatch_action = OctoEverywhereDetectionService._dispatch_action.__get__(detector)
+        http_client.post.side_effect = [
+            httpx.Response(200, json=context_payload()),
+            httpx.Response(200, json=process_payload(PauseSuggested=True)),
+        ]
+        with patch(
+            "backend.app.services.octoeverywhere_actions.execute_action", side_effect=RuntimeError("action failed")
+        ):
+            await poll_once(detector, settings(action="pause"))
+        assert detector._actions == set()
+        assert detector._states[1].action_fired is True
+        assert detector.get_status()["last_error"] == "OctoEverywhere could not execute the configured failure action."
+
     @pytest.mark.parametrize("change", ["print_ended", "disabled", "key_changed", "account_blocked"])
     async def test_cancel_during_action_lookup_prevents_hardware_control(
         self, detector, http_client, manager, clock, change
@@ -2528,6 +2788,7 @@ class TestLifecycle:
 
         # Exercise the real action dispatcher: its printer-name database lookup
         # awaits before pause/power-off, which used to leave a cancellation gap.
+        detector._prepare_action = OctoEverywhereDetectionService._prepare_action.__get__(detector)
         detector._dispatch_action = OctoEverywhereDetectionService._dispatch_action.__get__(detector)
         http_client.post.side_effect = [
             httpx.Response(200, json=context_payload()),