mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-23 16:36:23 +00:00
fix(themes): re-affirming the active skin repaints surfaces that missed the activation
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).
This commit is contained in:
parent
3459461663
commit
9dbad81077
4 changed files with 112 additions and 10 deletions
|
|
@ -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 })
|
||||
|
||||
|
|
|
|||
|
|
@ -29,10 +29,20 @@ 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 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)
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue