Skip to content

fix(desktop): restore persisted backend skin after gateway registers it - #76648

Open
sarthak-707 wants to merge 1 commit into
NousResearch:mainfrom
sarthak-707:fix/backend-skin-persist-restart
Open

fix(desktop): restore persisted backend skin after gateway registers it#76648
sarthak-707 wants to merge 1 commit into
NousResearch:mainfrom
sarthak-707:fix/backend-skin-persist-restart

Conversation

@sarthak-707

@sarthak-707 sarthak-707 commented Aug 2, 2026

Copy link
Copy Markdown

What does this PR do?

Fixes the desktop app snapping back to the default theme on every restart when the chosen theme is a backend skin (a YAML skin under $HERMES_HOME/skins, announced by the gateway, e.g. skin: catppuccin in config.yaml).

Root cause: backend skins are announced on gateway.ready, which fires after the ThemeProvider has already resolved the persisted theme choice. At that point normalizeSkin can't resolve the stored name yet, so it falls back to the default. Nothing re-reads the persisted choice once the skin registers, so the user's pick is stored but never painted again.

The fix re-reads the persisted skin whenever the backend theme registry grows and adopts it when it resolves to something we're not already showing. A functional setState keeps reference identity on the no-op path, so unrelated registry churn doesn't re-render the themed tree.

Related: #75374 fixes the same class of bug for plugin-contributed themes (contrib registry). This PR covers the backend-skin store; the two registries don't overlap.

Related Issue

No issue filed — found while debugging on Fedora. Searched open PRs before opening; #75374 is the closest existing one and covers the contributed-theme path, not backend skins.

Type of Change

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

Changes Made

  • apps/desktop/src/themes/context.tsx: new effect keyed on backendThemes + profileKey that re-resolves the persisted skin name whenever the backend registry grows
  • apps/desktop/src/themes/context.test.tsx: regression test reproducing the race (persisted backend skin → boot falls back to default → gateway seed registers the skin → assert repaint)

How to Test

Reproduction: set skin: catppuccin (or any skin in $HERMES_HOME/skins) in config.yaml, pick it in Appearance, restart the app. Without the fix the app paints the default on every launch; with it the choice sticks.

Verification done:

  1. New regression test fails on main without the fix, passes with it (red-green confirmed)
  2. Full themes suite green: 8 files / 67 tests
  3. Rebuilt the packaged desktop app and confirmed the fix is in the shipped bundle

Platform tested: Fedora 44 x86_64, packaged Electron build.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass — N/A: change is in the desktop app (TypeScript); ran the vitest themes suite instead (8 files / 67 tests green)
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: Fedora 44 x86_64

Documentation & Housekeeping

  • N/A for all: no new config keys, no architecture changes, no tool behavior changes, no new skills

Notes

Authored with AI assistance (Hermes Agent); the diff, reproduction, and test were verified manually before submitting.

Backend skins (YAML skins under $HERMES_HOME/skins, e.g. the catppuccin
skin referenced from config.yaml) are announced by the gateway on
gateway.ready, which fires after the ThemeProvider has already resolved
the persisted theme choice. normalizeSkin can't resolve the name yet, so
it silently falls back to the default and nothing re-reads the persisted
choice once the skin registers. The user's pick is stored, but the app
paints the default on every restart.

Re-read the persisted skin whenever the backend theme registry grows and
adopt it if it now resolves to something we're not already showing. The
functional setState preserves reference identity on the no-op path, so
unrelated registry churn never re-renders the themed tree.
@alt-glitch alt-glitch added type/bug Something isn't working comp/desktop Electron desktop app (apps/desktop/*) area/config Config system, migrations, profiles P3 Low — cosmetic, nice to have needs-decision Awaiting maintainer decision before any implementation labels Aug 2, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related: #71447 repairs persisted backend-skin restart behavior by retaining known skin names before gateway registration, while this PR re-adopts the persisted choice when the backend registry registers it. #75374 covers the separate contributed-theme registry. These are competing semantics, not duplicate patches.

@teknium1

teknium1 commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

Thanks for the focused regression fix. The premise holds on current origin/main (fc61608a17d6): normalizeSkin() rejects a persisted name until it is present in the live registry (apps/desktop/src/themes/context.tsx:47-48), while ThemeProvider initializes that normalized value at mount and does not revisit it for backend registry changes (apps/desktop/src/themes/context.tsx:338-352). Meanwhile gateway.ready intentionally registers the skin with apply: false (apps/desktop/src/app/session/hooks/use-message-stream/gateway-event.ts:287-290).

The effect in 7e9b2c4fc54e re-resolves the persisted value when $backendThemes publishes, and the added test exercises the unavailable-at-boot → gateway seed → repaint sequence. git diff f5ca0e2f..origin/main -- apps/desktop/src/themes/context.tsx apps/desktop/src/themes/context.test.tsx is empty, so this should salvage cleanly onto current main.

This is an automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Aug 2, 2026
@GottZ

GottZ commented Aug 3, 2026

Copy link
Copy Markdown

This was generated by AI during triage.

Summary

Three PRs address two related backend-skin startup failures: #71447 and #76648 both restore a previously persisted backend skin after delayed gateway registration, while #74018 adopts the backend-configured skin when a profile has no persisted Desktop preference and also corrects profile-scoped event routing.

Related pull requests

Duplicates

#71447 and #76648 address the same #71446 restart failure through competing implementations; treat #71447 as superseded by and duplicate of #76648. #74018 is not a duplicate because it addresses first-use adoption for #73987 and broader profile-scoped synchronization.

Suggested consolidation

Keep #76648 open with a salvage path: preserve its registry-triggered persisted-skin re-resolution and focused regression test. Close #71447 as duplicate of #76648 because its known-name cache leaves the contributor-identified junk-value and profile/runtime-plugin gaps unresolved, despite its keep_open review. Keep #74018 open separately for #73987, retaining its preference-presence guard and profile-scoped synchronization and routing work.

Complex graph

flowchart LR
    classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
    classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
    classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
    classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
    classDef best stroke-width:3px,stroke:#b45309
    classDef target stroke-width:3px,stroke:#4338ca
    I71446(["issue #71446 (open)"])
    subgraph Dup71447 ["PRs duplicating each other"]
        P71447["PR #71447 (open)"]
        P76648["PR #76648 (open)"]
    end
    P76648 -->|best fix| I71446
    class I71446 open
    class P71447 open
    class P76648 open
    class P71447 best
    class P76648 best
    class P76648 target
    click I71446 "https://github.com/NousResearch/hermes-agent/issues/71446"
    click P71447 "https://github.com/NousResearch/hermes-agent/pull/71447"
    click P76648 "https://github.com/NousResearch/hermes-agent/pull/76648"
Loading

Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).

Cross-PR triage: Reviewed 3 pull requests and 2 issues in this complex. Each diff was read against this issue; Assessment working set: 49 kB of PR diffs, 12 kB of issue/PR text, 7 kB of discussion (7 comments), 7 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

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

Labels

area/config Config system, migrations, profiles comp/desktop Electron desktop app (apps/desktop/*) needs-decision Awaiting maintainer decision before any implementation P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants