Skip to content

fix(gateway): reaffirm YAML skins on connect via skin.changed - #84803

Open
Zirillian wants to merge 1 commit into
NousResearch:mainfrom
Zirillian:fix/yaml-skin-connect-reaffirm
Open

fix(gateway): reaffirm YAML skins on connect via skin.changed#84803
Zirillian wants to merge 1 commit into
NousResearch:mainfrom
Zirillian:fix/yaml-skin-connect-reaffirm

Conversation

@Zirillian

Copy link
Copy Markdown

What does this PR do?

Fixes custom YAML skins (~/.hermes/skins/*.yaml, e.g. mythos) resetting to nous after every Desktop restart, without requiring a Desktop client rebuild.

gateway.ready seeds the skin with apply: false so a reconnect cannot stomp a built-in Appearance pick. That same rule leaves user YAML skins stuck: normalizeSkin rejects the persisted name until the seed lands, then never applies it. Built-ins already persist because they resolve on the first frame.

This PR sends a follow-up skin.changed only when the resolved skin is an on-disk user YAML (not a Desktop built-in). Current Desktop clients already treat seed-only + same-name skin.changed as missed-activation recovery (see backend-sync tests / #69533).

This is not another renderer-only persist PR. It is the gateway half that makes already-shipped Desktop binaries recover. Renderer-only PRs (#71447, #76648, #82483, #82951, #74018) still need a new Desktop build to help remote/Windows clients.

Verified on Windows Desktop (official binary) + Linux isolated hermes serve --isolated: reopen paints nous briefly, then auto-switches to the YAML skin with no Appearance click.

Related Issue

Fixes #71446

Also related (same class, first-use / renderer): #73987

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • tui_gateway/server.pyshould_reaffirm_yaml_skin_on_connect(); true only for on-disk YAML names that are not Desktop built-ins (nous, midnight, ember, mono, cyberpunk, slate, default)
  • tui_gateway/ws.py — after a successful gateway.ready seed + register_live_transport, emit skin.changed when the helper is true
  • tests/tui_gateway/test_protocol.py — helper: mythos True; nous/slate False; missing file False; {} / None False

How to Test

  1. hermes config set display.skin mythos (or any name with ~/.hermes/skins/<name>.yaml)
  2. Fully quit Desktop (tray → Quit), recycle leftover hermes serve --isolated if needed, reopen and reconnect
  3. Before: stays on nous until you re-pick
  4. After: brief nous (cold-boot fallback), then auto-switch to the YAML skin. No Appearance click.
  5. Pick a built-in (slate) in Appearance, restart — still slate. This PR must not reaffirm built-ins.

Unit:

pytest tests/tui_gateway/test_protocol.py::test_should_reaffirm_yaml_skin_on_connect -q -o addopts=

Sabotage check: helper forced return False → that test fails; restore → pass.

Checklist

Code

  • I've read the Contributing Guide
  • Conventional Commits (fix(gateway):)
  • Searched existing PRs — several renderer persist PRs are open; this one is gateway-only and complementary
  • PR contains only this fix
  • Targeted pytest (new test + test_unregister_live_transport_stops_delivery + test_skin_live_switch_end_to_end) passed. Full pytest tests/ -q not run in this checkout (no project pytest in the live venv until this run; CI will cover the rest)
  • Added tests
  • Tested on Linux host (Ubuntu) + Windows remote Desktop official binary against Linux isolated serve

Documentation & Housekeeping

  • N/A — no config keys, no docs/architecture change, no tool schema change
  • Cross-platform: Windows official client already handles skin.changed; no Windows rebuild required for this half

Why not change gateway.ready to apply: true?

That would stomp a user who picked Slate in Appearance while display.skin is still a YAML name. The seed contract stays apply: false. Only YAML skins get the extra skin.changed.

Desktop gateway.ready seeds skins with apply=false so a reconnect
cannot stomp a built-in Appearance pick. That same rule leaves user
YAML skins (not in Desktop BUILTIN_THEMES) stuck on nous after every
cold boot: normalizeSkin rejects the persisted name until the seed
lands, then never applies it.

Send a follow-up skin.changed for on-disk YAML skins only. Existing
Desktop clients already treat seed-only + same-name changed as
missed-activation recovery. Built-ins are skipped.

Fixes NousResearch#71446
@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/tui Terminal UI (ui-tui/ + tui_gateway/) labels Aug 12, 2026
@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference, author can ignore or act on any point.

fix(gateway): reaffirm YAML skins on connect via skin.changed

  1. tui_gateway/server.py _DESKTOP_BUILTIN_SKINS — this frozenset duplicates the built-in theme names from apps/desktop/src/themes/presets.ts (BUILTIN_THEMES), across two languages and two repos of drift. If a theme is added/removed on the desktop side, this list silently goes stale and a "built-in" pick would start being reaffirmed (or a YAML skin stops being reaffirmed). Consider deriving it from a shared source, or adding a cross-check test that greps the presets file for the built-in names (with a comment in both places pointing at each other).
  2. should_reaffirm_yaml_skin_on_connect builds f"{name}.yaml" from the skin payload's name without sanitization. It's only used for an is_file() existence check (no open/read/write), so impact is limited to an existence oracle, but a name containing / or .. would probe an arbitrary path under _watcher_home(). A Path(name).name guard or rejecting non-filename characters is cheap.
  3. tui_gateway/ws.py — the reaffirm skin.changed is written unconditionally on every connect for a YAML skin. The payload is the connect-time seed; if the server-side skin changed between the seed read and this write there could be a brief stale repaint, but the existing missed-activation recovery handles the common case. Minor: consider gating the write to only when the peer's stored skin (if the protocol reports it) differs, to avoid needless repaints on reconnect storms.
  4. The docstring explains the recovery semantics well — one suggestion: also cover the case where the YAML file is deleted after the first connect (the reaffirm check will start returning false and the desktop will stay on the last applied skin, which is reasonable — but the behavior is currently only implicitly defined by the file check).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/tui Terminal UI (ui-tui/ + tui_gateway/) P3 Low — cosmetic, nice to have type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(desktop): custom backend skins don't persist across restarts

3 participants