From 345946166303724368089f1d5a1dcf52abfd5265 Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Wed, 22 Jul 2026 12:50:18 -0500 Subject: [PATCH 1/3] fix(themes): broadcast live skin.changed to WS surfaces (desktop/dashboard), not just stdio MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The cross-surface theme SDK's live-repaint relies on a gateway skin watcher that polls config and emits skin.changed on any move. But that emit is session-less and fires from a background thread, so write_json fell through its (session-transport -> contextvar -> stdio) ladder to the module stdio transport — which only reaches the stdio TUI (tee'd to the dashboard WS publisher). WS clients (the desktop app, dashboard chat) never got it, so 'Hermes themes itself' repainted the CLI/TUI but not the GUI. Add a live-transport registry (one entry per connected WS peer, maintained by handle_ws) and a _broadcast_global_event primitive that fans session-less announcements out to every connected client, falling back to write_json when none are registered (stdio path unchanged). Route both skin.changed emits (watcher + the /skin RPC) through it, so a skin switch from any surface repaints all of them. Backend-only; desktop already handles skin.changed and does not drop session-less events. --- tests/test_tui_gateway_ws.py | 22 ++++++ tests/tui_gateway/test_protocol.py | 116 +++++++++++++++++++++++++++++ tui_gateway/server.py | 71 +++++++++++++++++- tui_gateway/ws.py | 6 ++ 4 files changed, 211 insertions(+), 4 deletions(-) diff --git a/tests/test_tui_gateway_ws.py b/tests/test_tui_gateway_ws.py index 1a9b37259c0..782073a05c9 100644 --- a/tests/test_tui_gateway_ws.py +++ b/tests/test_tui_gateway_ws.py @@ -127,6 +127,28 @@ def test_ws_disconnect_preserves_and_repoints_reconnectable_session(monkeypatch) server._sessions.clear() +def test_ws_connection_registers_then_disconnect_unregisters_live_transport(monkeypatch): + """A connected client must be tracked in the live-transport registry so a + session-less global broadcast (skin.changed from the background watcher) + reaches it, and dropped on disconnect so no stale write targets a dead peer. + This is the WS half of the cross-surface live-theme fix.""" + server._sessions.clear() + server._live_transports.clear() + seen = {} + try: + _run_disconnect( + monkeypatch, + lambda t: seen.__setitem__("registered", t in server._live_transports), + ) + # Seeded at receive_text time — i.e. after gateway.ready registered it. + assert seen["registered"] is True + # handle_ws's finally must have unregistered it. + assert not server._live_transports + finally: + server._sessions.clear() + server._live_transports.clear() + + def test_ws_write_loop_stall_does_not_latch_transport(monkeypatch): """A write that times out because the event loop is stalled (GIL-heavy agent turn) must NOT latch the transport closed — the frame is already diff --git a/tests/tui_gateway/test_protocol.py b/tests/tui_gateway/test_protocol.py index 864b1d8e61a..4477cd2ca89 100644 --- a/tests/tui_gateway/test_protocol.py +++ b/tests/tui_gateway/test_protocol.py @@ -41,6 +41,7 @@ def server(): mod._sessions.clear() mod._pending.clear() mod._answers.clear() + mod._live_transports.clear() @pytest.fixture() @@ -2161,3 +2162,118 @@ def test_broadcast_skin_if_changed_on_any_signature_move(server, monkeypatch): server._broadcast_skin_if_changed() assert [ev for ev, _ in emitted] == ["skin.changed"] * 3 + + +# ── global-event broadcast (session-less events reach every WS client) ── + + +class _RecordingTransport: + """Minimal Transport stand-in that records the frames written to it.""" + + def __init__(self) -> None: + self.frames: list[dict] = [] + + def write(self, obj: dict) -> bool: + self.frames.append(obj) + return True + + def close(self) -> None: + pass + + +def test_broadcast_global_event_reaches_registered_transports_from_background_thread(capture): + """The core of the bug: a session-less event emitted from a thread with no + contextvar-bound transport (the skin watcher) must still reach connected WS + clients. Before the registry it fell through to stdout and the desktop GUI + never repainted.""" + server, buf = capture + a, b = _RecordingTransport(), _RecordingTransport() + server.register_live_transport(a) + server.register_live_transport(b) + + # Emit from a background thread — no request context, no contextvar binding, + # exactly like the skin watcher loop. + t = threading.Thread( + target=server._broadcast_global_event, + args=("skin.changed", {"name": "synthwave", "colors": {"background": "#1a1030"}}), + ) + t.start() + t.join(timeout=2) + + for transport in (a, b): + assert len(transport.frames) == 1 + params = transport.frames[0]["params"] + assert params["type"] == "skin.changed" + assert params["session_id"] == "" # session-less: a surface-global announce + assert params["payload"]["name"] == "synthwave" + + # It must NOT have leaked onto stdout when live transports exist. + assert buf.getvalue() == "" + + +def test_broadcast_global_event_falls_back_to_stdio_without_transports(capture): + """With no client registered (the stdio TUI path, whose stdio transport is + tee'd to the dashboard WS publisher, and tests), broadcasting still emits via + write_json so that surface is unchanged.""" + server, buf = capture + assert not server._live_transports + + server._broadcast_global_event("skin.changed", {"name": "midnight"}) + + frame = json.loads(buf.getvalue()) + assert frame["params"]["type"] == "skin.changed" + assert frame["params"]["session_id"] == "" + assert frame["params"]["payload"]["name"] == "midnight" + + +def test_unregister_live_transport_stops_delivery(capture): + """A disconnected peer (unregistered in the ws finally block) receives nothing + — and a stale write is never attempted against its closed socket.""" + server, buf = capture + a = _RecordingTransport() + server.register_live_transport(a) + server.unregister_live_transport(a) + + server._broadcast_global_event("skin.changed", {"name": "x"}) + + assert a.frames == [] + # No live transports left → fell back to stdio. + assert json.loads(buf.getvalue())["params"]["type"] == "skin.changed" + + +def test_broadcast_global_event_survives_a_wedged_peer(capture): + """One broken transport must never starve the others (or the watcher thread).""" + server, _buf = capture + + class _Boom(_RecordingTransport): + def write(self, obj): + raise RuntimeError("peer gone") + + boom, good = _Boom(), _RecordingTransport() + server.register_live_transport(boom) + server.register_live_transport(good) + + server._broadcast_global_event("skin.changed", {"name": "x"}) + + assert len(good.frames) == 1 # the healthy peer still got it + + +def test_skin_change_broadcasts_to_every_connected_client(server, monkeypatch): + """End-to-end intent: a skin move repaints ALL connected surfaces, not just + the one that triggered it — the whole point of the cross-surface theme SDK.""" + desktop, dashboard = _RecordingTransport(), _RecordingTransport() + server.register_live_transport(desktop) + server.register_live_transport(dashboard) + + sigs = iter([("default", 1.0), ("synthwave", 2.0)]) + monkeypatch.setattr(server, "_last_skin_sig", None, raising=False) + monkeypatch.setattr(server, "_skin_sig", lambda: next(sigs)) + monkeypatch.setattr(server, "resolve_skin", lambda: {"name": "synthwave", "colors": {}}) + + server._broadcast_skin_if_changed() # first move → default + server._broadcast_skin_if_changed() # second move → synthwave + + for transport in (desktop, dashboard): + types = [f["params"]["type"] for f in transport.frames] + assert types == ["skin.changed", "skin.changed"] + assert transport.frames[-1]["params"]["payload"]["name"] == "synthwave" diff --git a/tui_gateway/server.py b/tui_gateway/server.py index 054f1747f8f..923a216c145 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -1219,6 +1219,66 @@ def _emit(event: str, sid: str, payload: dict | None = None): write_json({"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() +_live_transports_lock = threading.Lock() + + +def register_live_transport(transport) -> None: + """Track a connected client transport for global broadcasts. Idempotent.""" + if transport is None: + return + with _live_transports_lock: + _live_transports.add(transport) + + +def unregister_live_transport(transport) -> 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. + """ + with _live_transports_lock: + targets = list(_live_transports) + + if not targets: + _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} + 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). + logger.debug("global-event broadcast write failed type=%s", event, exc_info=True) + + _compute_host_supervisor = None _compute_host_supervisor_lock = threading.Lock() @@ -2477,7 +2537,7 @@ def _broadcast_skin_if_changed() -> None: return _last_skin_sig = sig try: - _emit("skin.changed", "", resolve_skin()) + _broadcast_global_event("skin.changed", resolve_skin()) except Exception: pass @@ -12137,9 +12197,12 @@ def _(rid, params: dict) -> dict: _write_config_key(f"display.{key}", value) nv = value if key == "skin": - _emit("skin.changed", "", resolve_skin()) - # Keep the reconcile baseline in sync so the per-tool check - # doesn't re-broadcast the skin the /skin RPC just applied. + # 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. + _broadcast_global_event("skin.changed", resolve_skin()) _note_skin_broadcast() resp = {"key": key, "value": nv} if key == "personality": diff --git a/tui_gateway/ws.py b/tui_gateway/ws.py index b61ef130cc4..e50e4456d0b 100644 --- a/tui_gateway/ws.py +++ b/tui_gateway/ws.py @@ -329,6 +329,11 @@ 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. + server.register_live_transport(transport) if not ready_ok: disconnect_reason = "ready_send_failed" send_failures += 1 @@ -429,6 +434,7 @@ async def handle_ws(ws: Any) -> None: reaped_sessions = 0 detached_sessions = 0 if transport is not None: + server.unregister_live_transport(transport) transport.close() # Reap sessions this transport owned (close_on_disconnect sidecar From 9dbad8107792f19ef5d8d73bc505ae6944715c59 Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Wed, 22 Jul 2026 13:29:56 -0500 Subject: [PATCH 2/3] fix(themes): re-affirming the active skin repaints surfaces that missed the activation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Real-world failure from dogfooding the live-theme flow: display.skin was already 'synthwave' in config, but the desktop never visibly applied it (the activation event predated the WS transport fix / the connect). The desktop's gateway.ready seed records the baseline WITHOUT painting (by design — never stomp the persisted desktop theme on connect), so it believed it was synced. Re-running 'hermes config set display.skin synthwave' then did nothing twice over: the watcher signature (name, skin-file mtime) hadn't moved, so no skin.changed fired; and even on an event, the desktop's name-equality guard blocked the apply against the seeded baseline. Two halves: - hermes_cli: setting display.skin touches the named skin file so the watcher signature always moves on an explicit set — a same-name re-affirm now broadcasts skin.changed like any real move. Built-ins (no file) are unaffected; a name switch already moves their signature. - desktop: track whether the synced baseline was actually APPLIED vs merely seeded at connect. A skin.changed matching a seed-only baseline is an intentional apply and repaints; once applied, repeat same-name events stay no-ops (protects a manual desktop-side theme switch from snap-back, incl. across a reconnect re-seed). --- apps/desktop/src/themes/backend-sync.test.ts | 30 ++++++++++++- apps/desktop/src/themes/backend-sync.ts | 30 +++++++++---- hermes_cli/config.py | 16 +++++++ tests/hermes_cli/test_set_config_value.py | 46 ++++++++++++++++++++ 4 files changed, 112 insertions(+), 10 deletions(-) diff --git a/apps/desktop/src/themes/backend-sync.test.ts b/apps/desktop/src/themes/backend-sync.test.ts index 64ed4f0e0b0..373333123a0 100644 --- a/apps/desktop/src/themes/backend-sync.test.ts +++ b/apps/desktop/src/themes/backend-sync.test.ts @@ -39,15 +39,41 @@ describe('ingestBackendSkin', () => { expect($pendingSkinApply.get()).toBe('forest') }) - it('seeds on connect so the first matching poll is a no-op, but a change applies', () => { + it('seed does not paint, but a later same-name skin.changed applies (missed-activation recovery)', () => { + // Connect while display.skin is already neon: seed records the baseline + // without painting (never stomp the persisted desktop theme on connect). ingestBackendSkin(skin('neon'), { apply: false }) // gateway.ready seed - ingestBackendSkin(skin('neon'), { apply: true }) // post-turn poll, unchanged expect($pendingSkinApply.get()).toBeNull() + // The activation event was missed (skin set while disconnected / backend + // restarted). Hermes re-affirms it — `hermes config set display.skin neon` + // or a `hermes skin set` recolor. That explicit event must repaint even + // though the name matches the seed. + ingestBackendSkin(skin('neon'), { apply: true }) + expect($pendingSkinApply.get()).toBe('neon') + + // Once applied, a repeat same-name event is a no-op again... + $pendingSkinApply.set(null) + ingestBackendSkin(skin('neon'), { apply: true }) + expect($pendingSkinApply.get()).toBeNull() + + // ...and a genuine switch still applies. ingestBackendSkin(skin('forest'), { apply: true }) // Hermes authored a new skin expect($pendingSkinApply.get()).toBe('forest') }) + it('a reconnect re-seed after a real apply does not downgrade the applied baseline', () => { + ingestBackendSkin(skin('neon'), { apply: true }) // applied for real + $pendingSkinApply.set(null) + + ingestBackendSkin(skin('neon'), { apply: false }) // reconnect: gateway.ready re-seed + ingestBackendSkin(skin('neon'), { apply: true }) // repeat event (e.g. in-place recolor) + + // Already painted once — the repeat must not re-apply (protects a manual + // desktop-side theme switch from being snapped back after a reconnect). + expect($pendingSkinApply.get()).toBeNull() + }) + it('never registers default in the backend store (desktop keeps its own palette)', () => { ingestBackendSkin(skin('default'), { apply: true }) diff --git a/apps/desktop/src/themes/backend-sync.ts b/apps/desktop/src/themes/backend-sync.ts index 57899640477..7ca1cc5ebf8 100644 --- a/apps/desktop/src/themes/backend-sync.ts +++ b/apps/desktop/src/themes/backend-sync.ts @@ -29,10 +29,20 @@ 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 drove onto the desktop. Guards two things: re-applying -// the same skin every post-turn poll, and snapping back after a manual switch — -// only a CHANGE from this value applies. `default` is the "no opinion" sentinel. -let lastSynced: 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. +let lastSynced: { applied: boolean; name: string } | null = null /** Test-only: reset the module's apply guard + registry between cases. */ export function __resetBackendSkinSync(): void { @@ -75,14 +85,18 @@ export function ingestBackendSkin(skin: HermesSkin | undefined | null, { apply } } if (!apply) { - // Connect-time seed: record the baseline so a later poll is a no-op. - lastSynced = name + // 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. + if (lastSynced?.name !== name || !lastSynced.applied) { + lastSynced = { applied: false, name } + } return } - if (name !== lastSynced) { - lastSynced = name + if (name !== lastSynced?.name || !lastSynced.applied) { + lastSynced = { applied: true, name } $pendingSkinApply.set(name) } } diff --git a/hermes_cli/config.py b/hermes_cli/config.py index 93dd86fad59..4fa44862420 100644 --- a/hermes_cli/config.py +++ b/hermes_cli/config.py @@ -8986,6 +8986,22 @@ 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. + 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() + except Exception: + pass # best-effort: the 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 # (lowercase, so it misses the .env api_keys list above) and would otherwise diff --git a/tests/hermes_cli/test_set_config_value.py b/tests/hermes_cli/test_set_config_value.py index ad9641300df..0e2cb6fd57c 100644 --- a/tests/hermes_cli/test_set_config_value.py +++ b/tests/hermes_cli/test_set_config_value.py @@ -630,3 +630,49 @@ class TestValidateConfigKey: from hermes_cli.config import _validate_config_key is_known, suggestion = _validate_config_key("agent._max_turns") assert not is_known, "Sub-key typo under a known top-level key must still be flagged" + + +# --------------------------------------------------------------------------- +# display.skin → touch the skin file (live re-affirm broadcast) +# --------------------------------------------------------------------------- + +class TestDisplaySkinTouch: + """Setting display.skin must bump the named skin file's mtime. + + The gateway's skin watcher broadcasts ``skin.changed`` on a signature move + of (active name, skin-file mtime). Re-affirming the already-configured skin + (`hermes config set display.skin X` while it is already X — the recovery + path when a surface missed the original activation) moves NEITHER part, so + without the touch the explicit apply is invisible to every live surface. + """ + + def test_reaffirming_same_skin_moves_the_watcher_signature(self, _isolated_hermes_home): + import os as _os + skins = _isolated_hermes_home / "skins" + skins.mkdir() + skin_file = skins / "synthwave.yaml" + skin_file.write_text("name: synthwave\ncolors:\n background: '#1a1030'\n") + # Age the file so an mtime bump is unambiguous even on coarse clocks. + _os.utime(skin_file, (1_000_000_000, 1_000_000_000)) + + set_config_value("display.skin", "synthwave") + first = skin_file.stat().st_mtime + assert first > 1_000_000_000 + + _os.utime(skin_file, (1_000_000_000, 1_000_000_000)) + set_config_value("display.skin", "synthwave") # same name, re-affirmed + assert skin_file.stat().st_mtime > 1_000_000_000 + + def test_builtin_or_missing_skin_file_is_fine(self, _isolated_hermes_home): + """Built-ins have no user file — the set must still succeed cleanly.""" + set_config_value("display.skin", "mono") + assert "skin: mono" in _read_config(_isolated_hermes_home) + + def test_touch_preserves_skin_file_contents(self, _isolated_hermes_home): + skins = _isolated_hermes_home / "skins" + skins.mkdir() + body = "name: neon\ncolors:\n ui_accent: '#ff33aa'\n" + (skins / "neon.yaml").write_text(body) + + set_config_value("display.skin", "neon") + assert (skins / "neon.yaml").read_text() == body From 39f72e4a5ec8ad0dbc42f4ca7d9730ca15e43919 Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Wed, 22 Jul 2026 14:07:18 -0500 Subject: [PATCH 3/3] 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 7ca1cc5ebf8..9a988730f10 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 4fa44862420..fd6de3a58db 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 923a216c145..e5299403160 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 e50e4456d0b..1dccc60f36a 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"