feat(screenshot): report theme changes instead of writing them back

The screenshot engine writes the saved frontend theme of the Home Assistant
user its token belongs to, which syncs to that user's live web and mobile
sessions (#1909). ha-mcp previously undid that with a snapshot/restore
bracket, but the restore was itself a frontend/set_user_data write from
tools annotated readOnlyHint: True -- which is why #1991 / PR #2014
disabled the bracket entirely, trading the protection away to keep the
annotation honest.

This keeps both. The guard still reads the saved theme before and after a
capture, but on a change it reports the previous value instead of writing
it, so the capture path issues no writes at all and the read-only
annotations stay accurate. Undoing the change moves to ha_manage_theme --
already annotated destructiveHint: True / readOnlyHint: False -- via two
new actions:

- get_engine_theme: read the engine account's per-user theme
- set_engine_theme: write it back, taking the value verbatim from the
  warning

These act on the engine account's per-user profile (frontend/set_user_data),
a different layer from the backend default that action='set' changes, and
they resolve the engine credential the way the guard does rather than using
ha-mcp's own client -- with a dedicated engine account those are different
users, and ha-mcp's own credential cannot reach the engine user's profile.

The warning also suggests giving the engine its own user and token, which
avoids the problem outright: the write then lands on an account nobody
looks at, and nothing is emitted.

Because nothing is written on the capture path, concurrent batches cannot
corrupt each other, so this needs no serialization -- and with it none of
the lock's failure modes. Detection is armed for every capture, not only
themed ones: the upstream fix sparing the no-parameter case
(balloob/home-assistant-addons#89) is unreleased as of Puppet 2.6.0.

Retained from the earlier attempt: the guard's WebSocket waits are bounded
(5s, vs send_command's 30s default) so a pre-render read cannot consume the
caller's MCP timeout, and the resolved credential carries the client's
verify_ssl override so self-signed instances are not silently undetectable.

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

Author: kingpanther13

docs/beta.md

@@ -177,18 +177,28 @@ token grants. If the token is missing or invalid, Puppet lands on the login
 page and (by its design) restarts; ha-mcp surfaces this as a clear "set the
 engine's access token" error rather than a silent failure.
 
-Puppet's theme and dark-mode renderer controls used to dispatch Home
-Assistant's `settheme` event on every cold render, which Home Assistant
-persisted on the frontend profile of the user whose token the engine runs with
-— and synced to that user's real web and mobile sessions, flipping a dark-mode
-user's whole UI to light on every screenshot (#1909). Recent Puppet versions
-fixed that cold-render dispatch, so ha-mcp's snapshot/restore bracket around
-each capture is now disabled (#1991); the guard code is retained so it can be
-switched back on if a future engine regression reintroduces the write. If you
-run an older Puppet build, update the app (or your self-hosted sidecar
-image) — older engines still persist the theme selection and will keep
-flipping it. A dedicated Puppet account remains a sound belt-and-suspenders
-setup. Language selection is local to Puppet's browser session.
+Puppet dispatches Home Assistant's `settheme` event on cold renders, which
+Home Assistant persists on the frontend profile of the user whose token the
+engine runs with — and syncs to that user's real web and mobile sessions,
+flipping that user's whole UI on every screenshot (#1909). Upstream stopped
+the dispatch for renders that request no theme
+(balloob/home-assistant-addons#89), but by its own title only for that case,
+and it is unreleased as of Puppet 2.6.0 — so on current releases every render
+writes.
+
+The screenshot and dashboard-get tools **detect** this and report it, but
+never write: they read the engine account's saved theme before and after the
+render and, when it changed, emit a warning naming the previous value. Undoing
+it is a separate, explicitly write-annotated call —
+`ha_manage_theme(action="set_engine_theme", value=...)` — so these tools stay
+honestly `readOnlyHint: True` (#1991). `ha_manage_theme(action=
+"get_engine_theme")` inspects the same value. Note this is the engine
+account's *per-user* profile, a different layer from the backend default that
+`action="set"` changes.
+
+**Give the engine its own Home Assistant user and long-lived token** and the
+problem disappears: the write lands on an account nobody looks at, and no
+warning is emitted. Language selection is local to Puppet's browser session.
 
 To change the Puppet engine app's own options (such as `keep_browser_open`)
 or to restart it, use `ha_manage_app`; the screenshot tools only render and

src/ha_mcp/dashboard_screenshot/capture.py

@@ -855,15 +855,12 @@ async def capture_dashboard_images(
     preset's native orientation. ``full_page`` is a compatibility alias for
     requesting the engine's native ``WIDTHxauto`` viewport.
 
-    The batch used to be bracketed by a :class:`ThemeGuard` that restored the
-    engine user's saved frontend theme when a cold render changed it (issue
-    #1909). That bracket is currently disabled (#1991): upstream Puppet no
-    longer dispatches ``settheme`` on cold renders, so there is nothing to undo.
-    The guard construction and the ``client`` / ``capture_warnings`` plumbing
-    are retained so the bracket can be re-enabled by uncommenting the
-    snapshot/restore calls if a future engine regression reintroduces the write;
-    while disabled ``capture_warnings`` simply stays empty and never affects the
-    captures themselves.
+    Each batch is bracketed by a :class:`ThemeGuard`, which reads the engine
+    user's saved frontend theme before and after rendering. It never writes:
+    when the render changed the theme it reports the previous value through
+    ``capture_warnings`` so the agent can restore it with the write-annotated
+    ``ha_manage_theme``, keeping this path honestly read-only (#1909, #1991).
+    Guard failures are non-fatal.
     """
     path = _validate_dashboard_path(dashboard_path)
     options = validate_capture_parameters(
@@ -888,16 +885,14 @@ async def capture_dashboard_images(
     mime_type = _MIME_TYPES[options.image_format]
     captures: list[DashboardImageCapture] = []
 
-    # ThemeGuard bracket — currently DISABLED (#1991). Stock Puppet used to
-    # dispatch a theme write into the authenticated frontend on cold renders,
-    # which Home Assistant persisted to the engine user's real profile (#1909);
-    # ha-mcp snapshotted before and restored after to undo it. Upstream Puppet
-    # has since fixed the cold-render settheme dispatch, so the bracket is no
-    # longer needed. The guard is still constructed (and the snapshot/restore
-    # calls kept below, commented out) so it can be re-enabled by uncommenting
-    # if a future engine regression reintroduces the write.
+    # ThemeGuard bracket. Puppet's settheme dispatch persists onto the engine
+    # token user's profile and syncs to that user's live sessions (#1909).
+    # Both reads here; the guard reports the change rather than undoing it, so
+    # this tool issues no writes and stays read-only (#1991). Armed for every
+    # capture, not just themed ones: the upstream fix that would spare the
+    # no-parameter case is unreleased as of Puppet 2.6.0.
     guard = ThemeGuard.for_capture(engine_target.addon_credential, client)
-    # await guard.take_snapshot()
+    await guard.take_snapshot()
     batch_error: ToolError | None = None
     try:
         async with httpx.AsyncClient(
@@ -977,7 +972,7 @@ async def capture_dashboard_images(
         # and its outcome can be attached to the error payload below.
         batch_error = exc
     finally:
-        # await guard.restore()  # ThemeGuard bracket disabled (#1991) — see above.
+        await guard.detect_change()
         if capture_warnings is not None:
             capture_warnings.extend(guard.warnings)
 

src/ha_mcp/dashboard_screenshot/theme_guard.py

@@ -1,43 +1,38 @@
-"""Snapshot and restore the engine user's saved frontend theme (issue #1909).
-
-NOTE: This guard is currently disabled at its call site (``capture.py``)
-because upstream Puppet fixed the cold-render ``settheme`` dispatch that made
-the bracket necessary (#1991). The code here is retained unchanged so the
-bracket can be re-enabled by uncommenting the snapshot/restore calls in
-``capture.py`` if a future engine regression reintroduces the write.
-
-Stock Puppet dispatches Home Assistant's ``settheme`` event on every
-cold-browser render — its ``dark`` query flag is presence-based, so "not
-requested" reaches the frontend as an explicit "light". Home Assistant
-persists that selection server-side per user (``frontend/set_user_data``,
-key ``"theme"``) and syncs it to every session of the user whose long-lived
-token the engine runs with. A plain screenshot call therefore flips a
-dark-mode user's real web and mobile UI to light.
-
-ha-mcp cannot suppress the engine's write, so every capture batch is
-bracketed instead: read the engine user's saved theme before rendering and
-write it back afterwards when the render changed it (an unchanged value is
-never rewritten).
+"""Detect when the screenshot engine changes the engine user's saved theme.
+
+Puppet dispatches Home Assistant's ``settheme`` event on cold-browser
+renders. Its ``dark`` query flag is presence-based, so "not requested"
+reaches the frontend as an explicit "light". Home Assistant persists that
+selection server-side per user (``frontend/set_user_data``, key ``"theme"``)
+and syncs it to every session of the user whose long-lived token the engine
+runs with, so a screenshot flips that user's real web and mobile UI (#1909).
+
+Upstream stopped the dispatch for renders requesting nothing
+(balloob/home-assistant-addons#89), but by its own title only for that case,
+and it is unreleased as of Puppet 2.6.0 -- so on current releases every
+render writes.
+
+**This guard never writes.** It reads the saved theme before the batch and
+again afterwards, and when the render changed it, reports the previous value
+so the agent can restore it with ``ha_manage_theme`` -- a tool correctly
+annotated as a write. That keeps the screenshot and dashboard-get tools
+honestly ``readOnlyHint: True`` (#1991, PR #2014) while still surfacing the
+damage. Because nothing is written, concurrent batches cannot corrupt each
+other and no serialization is needed.
+
+Pointing the engine at its own dedicated Home Assistant user avoids the
+problem outright: the write then lands on an account nobody looks at.
 
 Credential resolution mirrors engine discovery:
 
-- **HA OS / Supervised** — the Puppet add-on's own ``access_token`` and
+- **HA OS / Supervised** -- the Puppet add-on's own ``access_token`` and
   ``home_assistant_url`` options, taken from the Supervisor add-on info that
   engine discovery already fetches. The token lives only in process memory
-  for the duration of one capture batch and is never logged or returned.
-- **Docker / standalone / OAuth / embedded** — ha-mcp's direct Home
-  Assistant credentials. These protect the user whenever the sidecar engine
-  runs with a token for the same user (the common single-user setup).
-- Anything else (e.g. Supervisor-proxy auth with no discoverable engine
-  token) — the guard stays inactive and captures behave as before.
-
-Guard failures are always non-fatal: screenshots must keep working even
-when the theme cannot be protected. A snapshot or restore that was
-*attempted* but failed surfaces as a tool-response warning.
-
-Known limit: if a real session changes the user's theme during the few
-seconds of a capture batch, the restore reverts that change too — the guard
-cannot tell the engine's write apart from a concurrent human one.
+  and is never logged or returned.
+- **Docker / standalone / OAuth / embedded** -- ha-mcp's direct Home
+  Assistant credentials, which match whenever the engine runs with a token
+  for the same user (the common single-user setup).
+- Anything else -- detection stays inactive and captures behave as before.
 """
 
 from __future__ import annotations
@@ -75,6 +70,10 @@
 # restore, and let the late write survive).
 RESTORE_SETTLE_SECONDS = 1.0
 
+# The guard runs before the engine is contacted and must never eat the
+# caller's MCP timeout window; send_command otherwise waits 30s by default.
+COMMAND_TIMEOUT_SECONDS = 5.0
+
 
 @dataclass(frozen=True, slots=True)
 class EngineCredential:
@@ -88,6 +87,7 @@ class EngineCredential:
 
     url: str
     token: str
+    verify_ssl: bool | None = None
 
 
 def addon_credential_from_options(
@@ -119,7 +119,15 @@ def _client_credential(client: Any) -> EngineCredential | None:
     token = str(getattr(client, "token", "") or "").strip()
     if not base_url.startswith(("http://", "https://")) or not token:
         return None
-    return EngineCredential(url=base_url, token=token)
+    # Carry the client's own TLS setting: a direct client built with
+    # verify_ssl=False (self-signed HA) must not fall back to the global
+    # default, or every session fails and no change is ever detected.
+    verify_ssl = getattr(client, "verify_ssl", None)
+    return EngineCredential(
+        url=base_url,
+        token=token,
+        verify_ssl=verify_ssl if isinstance(verify_ssl, bool) else None,
+    )
 
 
 @dataclass
@@ -135,6 +143,7 @@ class ThemeGuard:
     warnings: list[str] = field(default_factory=list)
     _snapshot: Any = None
     _snapshot_taken: bool = False
+    changed_from: Any = None
 
     @classmethod
     def for_capture(
@@ -157,7 +166,11 @@ async def _session(self) -> AsyncIterator[HomeAssistantWebSocketClient]:
         from ..client.websocket_client import HomeAssistantWebSocketClient
 
         assert self.credential is not None
-        ws = HomeAssistantWebSocketClient(self.credential.url, self.credential.token)
+        ws = HomeAssistantWebSocketClient(
+            self.credential.url,
+            self.credential.token,
+            verify_ssl=self.credential.verify_ssl,
+        )
         if not await ws.connect():
             reason = ws.last_connect_error
             detail = f": {reason}" if isinstance(reason, str) else ""
@@ -173,7 +186,9 @@ async def _session(self) -> AsyncIterator[HomeAssistantWebSocketClient]:
     async def _fetch_theme(ws: HomeAssistantWebSocketClient) -> Any:
         """Read the persisted ``theme`` frontend user-data value (may be None)."""
         response = await ws.send_command(
-            "frontend/get_user_data", key=THEME_USER_DATA_KEY
+            "frontend/get_user_data",
+            key=THEME_USER_DATA_KEY,
+            _wait_timeout=COMMAND_TIMEOUT_SECONDS,
         )
         payload = response.get("result") if isinstance(response, dict) else None
         return payload.get("value") if isinstance(payload, dict) else None
@@ -197,38 +212,79 @@ async def take_snapshot(self) -> None:
                 f"theme before rendering; if the render changed it, {_RESTORE_HINT}."
             )
 
-    async def restore(self) -> None:
-        """Write the snapshot back if the render changed it. Never raises."""
+    async def detect_change(self) -> None:
+        """Report -- never repair -- a theme the render changed. Never raises.
+
+        Writing the value back here would make the screenshot tools issue
+        ``frontend/set_user_data``, which is exactly what disqualifies them
+        from ``readOnlyHint: True``. Instead the previous value is surfaced
+        as a warning naming the write-annotated tool that can restore it.
+        """
         if not self._snapshot_taken or self.credential is None:
             return
         try:
             # Puppet's settheme dispatch happens during page navigation, but
-            # the frontend's resulting user-data write is asynchronous — let
+            # the frontend's resulting user-data write is asynchronous -- let
             # it land before reading (see RESTORE_SETTLE_SECONDS).
             await asyncio.sleep(RESTORE_SETTLE_SECONDS)
             async with self._session() as ws:
                 current = await self._fetch_theme(ws)
-                if current != self._snapshot:
-                    # A never-configured baseline must restore as {} rather
-                    # than null: live frontend sessions ignore a null
-                    # subscription push (they would stay flipped until
-                    # reload), while an empty settings object re-applies
-                    # default/auto behavior immediately and means the same
-                    # thing on the next frontend boot.
-                    restore_value = self._snapshot if self._snapshot is not None else {}
-                    await ws.send_command(
-                        "frontend/set_user_data",
-                        key=THEME_USER_DATA_KEY,
-                        value=restore_value,
-                    )
         except Exception as exc:
             logger.warning(
-                "Could not restore the screenshot engine user's saved theme "
+                "Could not re-read the screenshot engine user's saved theme "
                 "after rendering: %s",
                 exc,
             )
             self.warnings.append(
-                "The screenshot render may have changed the saved frontend "
-                "theme of the engine token's user and restoring it failed; "
+                "Could not check whether the screenshot render changed the "
+                "saved frontend theme of the engine token's user; "
                 f"{_RESTORE_HINT}."
             )
+            return
+        if current == self._snapshot:
+            return
+        self.changed_from = self._snapshot
+        # A never-configured baseline is restored as {} rather than null:
+        # live frontend sessions ignore a null subscription push (they stay
+        # flipped until reload), while an empty settings object re-applies
+        # default/auto behavior immediately and means the same thing on the
+        # next frontend boot.
+        restore_value = self._snapshot if self._snapshot is not None else {}
+        logger.info(
+            "Screenshot render changed the engine user's saved theme "
+            "(was %s, now %s); reporting for agent-side restore",
+            self._snapshot,
+            current,
+        )
+        self.warnings.append(
+            "The screenshot engine changed the saved frontend theme of the "
+            "account its token belongs to, which also changes that account's "
+            "live web and mobile sessions. This tool is read-only and will "
+            "not change it back. To restore it, call ha_manage_theme("
+            f"action='set_engine_theme', value={restore_value!r}). "
+            "To stop this happening at all, give the screenshot engine its "
+            "own Home Assistant user and long-lived token, so its writes "
+            "land on an account nobody looks at."
+        )
+
+
+async def read_engine_theme(credential: EngineCredential) -> Any:
+    """Read the engine user's saved ``theme`` frontend user-data value."""
+    guard = ThemeGuard(credential=credential)
+    async with guard._session() as ws:
+        return await ThemeGuard._fetch_theme(ws)
+
+
+async def write_engine_theme(credential: EngineCredential, value: Any) -> None:
+    """Write the engine user's saved ``theme`` frontend user-data value.
+
+    Only reached through ``ha_manage_theme``, which is annotated as a write.
+    """
+    guard = ThemeGuard(credential=credential)
+    async with guard._session() as ws:
+        await ws.send_command(
+            "frontend/set_user_data",
+            key=THEME_USER_DATA_KEY,
+            value=value,
+            _wait_timeout=COMMAND_TIMEOUT_SECONDS,
+        )