From 39f72e4a5ec8ad0dbc42f4ca7d9730ca15e43919 Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Wed, 22 Jul 2026 14:07:18 -0500 Subject: [PATCH] refactor(themes): DRY the event frame, type the transport registry, tighten comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _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 | 24 ++++------ hermes_cli/config.py | 21 ++++----- tui_gateway/server.py | 62 ++++++++++--------------- tui_gateway/ws.py | 6 +-- 4 files changed, 44 insertions(+), 69 deletions(-) diff --git a/apps/desktop/src/themes/backend-sync.ts b/apps/desktop/src/themes/backend-sync.ts index 7ca1cc5ebf8f..9a988730f10f 100644 --- a/apps/desktop/src/themes/backend-sync.ts +++ b/apps/desktop/src/themes/backend-sync.ts @@ -29,19 +29,12 @@ export const $backendThemes = atom>({}) /** One-shot skin name the ThemeProvider should switch to (it clears this). */ export const $pendingSkinApply = atom(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 } } diff --git a/hermes_cli/config.py b/hermes_cli/config.py index 4fa44862420d..fd6de3a58dbb 100644 --- a/hermes_cli/config.py +++ b/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 diff --git a/tui_gateway/server.py b/tui_gateway/server.py index 923a216c1453..e52994031609 100644 --- a/tui_gateway/server.py +++ b/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} diff --git a/tui_gateway/ws.py b/tui_gateway/ws.py index e50e4456d0b2..1dccc60f36a8 100644 --- a/tui_gateway/ws.py +++ b/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"