test(themes): deterministic engine-identity e2e, and own the disconnect task

Adds the deterministic no-engine coverage where it is actually reachable.
dashboard_screenshot_engine_url is a runtime-settable AdvancedField
resolved live per capture, so ha_dev_manage_settings can point it at an
explicit URL mid-run -- which forces resolve_engine() to return no
addon_credential, exactly the case where ha-mcp's own credential must not
be treated as the engine account. Both engine-theme actions are asserted to
refuse, and the setting is reset in a finally. This lives in
test_dev_mode_tools.py because the tool is registered only when
HAMCP_ENABLE_DEV_MODE is on, which the themes suite does not set; the
themes-suite test stays tolerant and asserts the structured-error contract.

The shielded close left its inner task pending after a timeout or
cancellation, still holding the socket and able to raise late with no owner.
The task is now created explicitly, cancelled when the bounded wait gives
up, and awaited with return_exceptions=True before the finally exits. The
regression test blocks disconnect() and asserts the task is settled rather
than left pending.

The themes e2e now asserts success is False explicitly before reading the
error fields: the previous shape returned early only on success is True, so
a response missing the field entirely could have passed on truthy error
values alone.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012Nm7tyA1nfxNCWFXaR3AxV

Author: kingpanther13

src/ha_mcp/dashboard_screenshot/theme_guard.py

@@ -238,19 +238,28 @@ async def _session(self) -> AsyncIterator[HomeAssistantWebSocketClient]:
         finally:
             # Best-effort: a real failure to close must not mask the original
             # exception (including the cancellation that triggered cleanup).
+            # Owned explicitly: a bare shield leaves the inner task pending
+            # after a timeout or cancellation, still holding the socket and
+            # able to raise late with nobody to receive it.
+            close = asyncio.ensure_future(ws.disconnect())
             try:
                 # shield: we are frequently here *because* of a cancellation
                 # (SESSION_TIMEOUT_SECONDS). A bare await would be cancelled
                 # at once and the socket would never actually close.
                 await asyncio.wait_for(
-                    asyncio.shield(ws.disconnect()),
-                    timeout=CLOSE_TIMEOUT_SECONDS,
+                    asyncio.shield(close), timeout=CLOSE_TIMEOUT_SECONDS
                 )
             except (Exception, TimeoutError) as close_error:
                 logger.debug(
                     "Ignoring error while closing the theme-guard session: %s",
                     close_error,
                 )
+            finally:
+                if not close.done():
+                    close.cancel()
+                # Retrieve the outcome so a late failure never surfaces as an
+                # orphaned "exception was never retrieved" warning.
+                await asyncio.gather(close, return_exceptions=True)
 
     @staticmethod
     async def _fetch_theme(ws: HomeAssistantWebSocketClient) -> Any:

tests/src/e2e/tools/test_dev_mode_tools.py

@@ -276,3 +276,63 @@ async def test_restart_unavailable_standalone(self, mcp_client_with_dev_mode):
         )
         assert result.get("success") is not True
         assert "standalone" in extract_error_message(result).lower()
+
+
+class TestEngineThemeIdentityGuard:
+    """Deterministic engine-account coverage, only reachable with dev mode.
+
+    ha_dev_manage_settings can set dashboard_screenshot_engine_url at runtime
+    (it is resolved live per capture, so no restart is needed). Pointing it at
+    an explicit URL forces resolve_engine() to return no addon_credential,
+    which is exactly the case where ha-mcp's own credential must NOT be
+    treated as the engine account -- so the refusal is deterministic here in a
+    way it cannot be in the themes suite, where dev mode is off.
+    """
+
+    async def test_explicit_engine_url_refuses_engine_theme_actions(
+        self, mcp_client_with_dev_mode
+    ):
+        set_result = await safe_call_tool(
+            mcp_client_with_dev_mode,
+            "ha_dev_manage_settings",
+            {
+                "action": "set",
+                "setting": "dashboard_screenshot_engine_url",
+                "value": "http://sidecar.example:10000",
+            },
+        )
+        assert set_result.get("success") is True, f"Could not set URL: {set_result}"
+
+        try:
+            read = await safe_call_tool(
+                mcp_client_with_dev_mode,
+                "ha_manage_theme",
+                {"action": "get_engine_theme"},
+            )
+            assert read.get("success") is False, (
+                f"Reading an unidentifiable engine account must fail: {read}"
+            )
+            assert "identified" in extract_error_message(read).lower(), (
+                f"Error should name the identity problem: {read}"
+            )
+
+            # The write refuses for the same reason, so a restore can never
+            # land on ha-mcp's own profile.
+            write = await safe_call_tool(
+                mcp_client_with_dev_mode,
+                "ha_manage_theme",
+                {
+                    "action": "set_engine_theme",
+                    "value": {"theme": "", "dark": False},
+                    "expected_current": None,
+                },
+            )
+            assert write.get("success") is False, (
+                f"Writing an unidentifiable engine account must fail: {write}"
+            )
+        finally:
+            await safe_call_tool(
+                mcp_client_with_dev_mode,
+                "ha_dev_manage_settings",
+                {"action": "reset", "setting": "dashboard_screenshot_engine_url"},
+            )

tests/src/e2e/workflows/themes/test_manage_theme.py

@@ -236,6 +236,9 @@ async def test_get_engine_theme_fails_actionably_without_an_engine(
             )
             return
 
+        assert data.get("success") is False, (
+            f"Expected an explicit failure, got: {data}"
+        )
         # Assert the structured-error contract rather than matching wording:
         # the message varies with WHY the engine is unavailable (not
         # configured, not installed, not started, unidentifiable account).

tests/src/unit/test_dashboard_screenshot_theme_guard.py

@@ -541,6 +541,40 @@ async def hang_connect(self: Any) -> bool:
         assert len(_FakeWsClient.instances) == 1
         assert _FakeWsClient.instances[0].disconnected is True
 
+    async def test_blocked_disconnect_is_cancelled_not_orphaned(
+        self, monkeypatch: Any
+    ) -> None:
+        """A disconnect that never completes must not outlive the cleanup."""
+        import ha_mcp.dashboard_screenshot.theme_guard as guard_module
+
+        entered = asyncio.Event()
+        tasks: list[Any] = []
+
+        async def hang_disconnect(self: Any) -> None:
+            entered.set()
+            await asyncio.sleep(3600)
+
+        real_ensure = asyncio.ensure_future
+
+        def tracking_ensure(coro: Any, **kw: Any) -> Any:
+            task = real_ensure(coro, **kw)
+            tasks.append(task)
+            return task
+
+        with monkeypatch.context() as patched:
+            patched.setattr(_FakeWsClient, "disconnect", hang_disconnect)
+            patched.setattr(guard_module, "CLOSE_TIMEOUT_SECONDS", 0.01)
+            patched.setattr(guard_module.asyncio, "ensure_future", tracking_ensure)
+            _FakeWsClient.user_data[THEME_USER_DATA_KEY] = dict(_DARK_THEME)
+            guard = ThemeGuard.for_capture(_PUPPET_CREDENTIAL, None)
+            await guard.take_snapshot()
+            await asyncio.wait_for(entered.wait(), timeout=1)
+
+        # The cleanup owned the blocked close and finished with it settled,
+        # rather than leaving it pending against a live socket.
+        assert tasks, "cleanup should have created a disconnect task"
+        assert all(t.done() for t in tasks), "disconnect task left pending"
+
 
 class TestNullExpectedCurrentGuard:
     """An explicit null expected_current must guard, not mean 'unguarded'."""