diff --git a/apps/desktop/src/themes/backend-sync.test.ts b/apps/desktop/src/themes/backend-sync.test.ts index 64ed4f0e0b03..373333123a0c 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 578996404770..7ca1cc5ebf8f 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 93dd86fad599..4fa44862420d 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 ad9641300df5..0e2cb6fd57cf 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