refactor(themes): DRY the event frame, type the transport registry, tighten comments

_emit and _broadcast_global_event were each building the JSON-RPC event
envelope — extract _event_frame and use it from both. Type the registry as
set[Transport] (protocol already imported), and cut comment bloat at the
call sites. No behavior change; suites stay green.

apps/desktop/src/themes/backend-sync.ts

@@ -29,19 +29,12 @@ export const $backendThemes = atom<Record<string, DesktopTheme>>({})
 /** One-shot skin name the ThemeProvider should switch to (it clears this). */
 export const $pendingSkinApply = atom<string | null>(null)
 
-// The last skin name we synced from the backend, plus whether we ever actually
-// APPLIED it (vs merely recorded it at connect time). Guards two things:
-// re-applying the same skin on every repeat event, and snapping back after a
-// manual desktop-side switch — once applied, only a name CHANGE applies again.
-//
-// `applied` matters for the recovery path: the connect seed records the
-// baseline without painting (so a fresh connect never stomps the user's
-// persisted desktop theme). If the activation event was missed (backend
-// restart, or the skin was activated while disconnected), the desktop believes
-// it is synced while visibly not themed. A later explicit `skin.changed` for
-// that SAME name — Hermes re-running `hermes config set display.skin X`, or
-// `hermes skin set` recoloring the active skin — is an intentional apply and
-// must repaint, not no-op against the seed.
+// Last skin name synced from the backend + whether it was ever APPLIED (vs
+// merely seeded at connect). Once applied, only a name change applies again —
+// no re-apply on repeat events, no snap-back after a manual desktop switch.
+// A `skin.changed` matching a seed-only baseline still applies: the seed
+// records without painting, so if the activation event was missed (backend
+// restart / disconnected), an explicit re-affirm must repaint, not no-op.
 let lastSynced: { applied: boolean; name: string } | null = null
 
 /** Test-only: reset the module's apply guard + registry between cases. */
@@ -85,9 +78,8 @@ export function ingestBackendSkin(skin: HermesSkin | undefined | null, { apply }
   }
 
   if (!apply) {
-    // Connect-time seed: record the baseline WITHOUT painting. Keep an earlier
-    // real apply's flag if a reconnect re-seeds the same name, so a post-
-    // reconnect repeat event doesn't re-apply over a manual desktop switch.
+    // Connect-time seed: record without painting. A reconnect re-seed keeps an
+    // earlier real apply's flag so repeat events can't override a manual switch.
     if (lastSynced?.name !== name || !lastSynced.applied) {
       lastSynced = { applied: false, name }
     }

hermes_cli/config.py

@@ -8986,21 +8986,18 @@ def set_config_value(key: str, value: str, force: bool = False):
     if env_var and key != "terminal.cwd":
         save_env_value(env_var, _terminal_env_value(value))
 
-    # Setting display.skin is an explicit "apply this skin NOW" — bump the skin
-    # file's mtime so the gateway's skin watcher sees a signature move even when
-    # the NAME is unchanged. Without this, re-affirming the already-configured
-    # skin (`hermes config set display.skin X` while display.skin is already X —
-    # the recovery path when a surface missed the original activation) is
-    # invisible to the watcher: its signature is (name, skin-file mtime) and
-    # neither part moved. Built-ins have no file; a name switch already moves
-    # the signature for them.
+    # Setting display.skin is an explicit "apply NOW" — bump the skin file's
+    # mtime so the gateway watcher's (name, mtime) signature moves even when the
+    # name is unchanged (re-affirming the active skin after a surface missed the
+    # original activation). Built-ins have no file; a name switch already moves
+    # their signature.
     if key == "display.skin" and isinstance(value, str) and value:
         try:
-            _skin_file = get_hermes_home() / "skins" / f"{value}.yaml"
-            if _skin_file.exists():
-                _skin_file.touch()
+            skin_file = get_hermes_home() / "skins" / f"{value}.yaml"
+            if skin_file.exists():
+                skin_file.touch()
         except Exception:
-            pass  # best-effort: the write above already succeeded
+            pass  # best-effort: the config write above already succeeded
 
     # Mask the echoed value when the (possibly nested) key is credential-shaped
     # — e.g. `hermes config set model.api_key cfut_...` routes to config.yaml

tui_gateway/server.py

@@ -1212,23 +1212,26 @@ def write_json(obj: dict) -> bool:
     return (current_transport() or _stdio_transport).write(obj)
 
 
-def _emit(event: str, sid: str, payload: dict | None = None):
-    params = {"type": event, "session_id": sid}
+def _event_frame(event: str, sid: str, payload: dict | None = None) -> dict:
+    params: dict = {"type": event, "session_id": sid}
     if payload is not None:
         params["payload"] = payload
-    write_json({"jsonrpc": "2.0", "method": "event", "params": params})
+    return {"jsonrpc": "2.0", "method": "event", "params": params}
 
 
-# Registry of every live client transport (one per connected WS peer). Populated
-# by tui_gateway.ws for the lifetime of each connection. This is the ONLY way a
-# session-less, surface-global announcement can reach WS clients: write_json only
-# routes to a session's transport (by id) or the request's contextvar-bound
-# transport, and a background thread has neither. See _broadcast_global_event.
-_live_transports: set = set()
+def _emit(event: str, sid: str, payload: dict | None = None):
+    write_json(_event_frame(event, sid, payload))
+
+
+# Live client transports, one per connected WS peer (maintained by tui_gateway.ws).
+# A session-less event from a background thread has neither a session transport
+# nor a contextvar binding, so write_json would drop it on stdio — this registry
+# is how such events reach WS clients at all. See _broadcast_global_event.
+_live_transports: set[Transport] = set()
 _live_transports_lock = threading.Lock()
 
 
-def register_live_transport(transport) -> None:
+def register_live_transport(transport: Transport | None) -> None:
     """Track a connected client transport for global broadcasts. Idempotent."""
     if transport is None:
         return
@@ -1236,27 +1239,18 @@ def register_live_transport(transport) -> None:
         _live_transports.add(transport)
 
 
-def unregister_live_transport(transport) -> None:
+def unregister_live_transport(transport: Transport | None) -> None:
     """Stop tracking a transport (call on disconnect). Idempotent."""
     with _live_transports_lock:
         _live_transports.discard(transport)
 
 
 def _broadcast_global_event(event: str, payload: dict | None = None) -> None:
-    """Fan a session-less, surface-global event out to EVERY connected client.
-
-    Session-scoped events route to their session's transport, and an in-request
-    emit rides the contextvar-bound transport. A *global* announcement like
-    ``skin.changed`` has no session id, and the emitter (the skin watcher) runs
-    on a background thread with no contextvar binding — so ``write_json`` would
-    fall through to the module stdio transport and never reach the WS clients
-    the desktop app and dashboard chat connect over. Broadcasting to the live
-    transport registry is what makes "Hermes themes itself, live, everywhere"
-    actually repaint the GUI and not just the stdio surfaces.
-
-    When no transports are registered (the stdio TUI path, whose ``_stdio_transport``
-    is tee'd to the dashboard WS publisher, and tests), fall back to ``write_json``
-    so that surface is unchanged.
+    """Fan a session-less, surface-global event (``skin.changed``) to every
+    connected client. Emitters like the skin watcher run on background threads
+    where ``write_json``'s ladder bottoms out at stdio and WS peers never see
+    the frame. No registered transports (stdio TUI, tests) → plain ``_emit``,
+    which that path already tees where it needs to go.
     """
     with _live_transports_lock:
         targets = list(_live_transports)
@@ -1265,17 +1259,13 @@ def _broadcast_global_event(event: str, payload: dict | None = None) -> None:
         _emit(event, "", payload)
         return
 
-    params: dict = {"type": event, "session_id": ""}
-    if payload is not None:
-        params["payload"] = payload
-    frame = {"jsonrpc": "2.0", "method": "event", "params": params}
+    frame = _event_frame(event, "", payload)
     for transport in targets:
         try:
             transport.write(frame)
         except Exception:
-            # A wedged/closed peer must never stall the others (or the watcher
-            # thread). Disconnect teardown unregisters it; a stale write here is
-            # harmless (WSTransport.write returns False once closed).
+            # One wedged peer must not stall the rest; disconnect teardown
+            # unregisters it.
             logger.debug("global-event broadcast write failed type=%s", event, exc_info=True)
 
 
@@ -12197,11 +12187,9 @@ def _(rid, params: dict) -> dict:
                 _write_config_key(f"display.{key}", value)
                 nv = value
                 if key == "skin":
-                    # Broadcast to EVERY connected surface, not just the client
-                    # that issued the RPC — a `/skin` from the desktop should
-                    # repaint an open dashboard/CLI too. _note_skin_broadcast()
-                    # then syncs the watcher baseline so the poll loop doesn't
-                    # re-broadcast the skin this RPC just applied.
+                    # Every connected surface repaints, not just the RPC's
+                    # client; then sync the watcher baseline so the poll loop
+                    # doesn't re-broadcast the skin this RPC just applied.
                     _broadcast_global_event("skin.changed", resolve_skin())
                     _note_skin_broadcast()
             resp = {"key": key, "value": nv}

tui_gateway/ws.py

@@ -329,10 +329,8 @@ async def handle_ws(ws: Any) -> None:
         if ready_ok:
             # Live-apply skins Hermes activates mid-conversation.
             server._ensure_skin_watcher()
-            # Track this peer so session-less global broadcasts (skin.changed
-            # from the background watcher) actually reach it — they carry no
-            # session id and the watcher thread has no contextvar-bound
-            # transport, so without this they'd never land on a WS client.
+            # Track this peer for session-less global broadcasts (skin.changed
+            # from the background watcher) — write_json can't route those.
             server.register_live_transport(transport)
         if not ready_ok:
             disconnect_reason = "ready_send_failed"