Files
hermes-webui/tests
1c4ea9c99a fix(appearance): don't let the boot script fabricate an explicit them… (#6808)
* fix(appearance): don't let the boot script fabricate an explicit theme choice

`syncSettings` in static/boot.js is careful to let the server win on a first
visit. Its comment says why:

    // empty (new-browser) state is indistinguishable from a user who chose
    // the defaults.  To avoid blocking server→client sync on first visit we
    // only let localStorage override the server when it carries an explicit
    // user-selectable theme value or a NON-DEFAULT skin.

The inline bootstrap in static/index.html defeats that. It resolves a theme with
`localStorage.getItem('hermes-theme')||'dark'` and then unconditionally writes the
result back:

    localStorage.setItem('hermes-theme',theme);localStorage.setItem('hermes-skin',skin);

On a brand-new browser that stores `hermes-theme=dark` before any request is made.
By the time syncSettings runs, `lsHasExplicitTheme` is true, so:

1. the server's `theme`/`skin` in SETTINGS_DEFAULTS (or settings.json) is ignored, and
2. the reconcile branch below POSTs the fabricated value back to /api/settings,
   overwriting the stored setting it was supposed to be reading.

The net effect is that the server-side appearance default is unreachable: a fresh
browser always lands on dark and then persists it server-side. Deployments that
set a different default in SETTINGS_DEFAULTS never see it applied.

Fix: only rewrite localStorage when there was already appearance state to
normalise. A fresh visit now leaves localStorage untouched, so `lsHasExplicitTheme`
is false and syncSettings takes the server value exactly as its comment intends.

Deliberately keeps the existing normalisation. The write is not pointless — it
canonicalises legacy names (`solarized` → `dark` + `poseidon`). Gating on "was
there any prior state" rather than per-key preserves that: with `hermes-theme=solarized`
and no `hermes-skin`, the derived skin is still written, so the mapping survives
the next load. A per-key guard would have silently dropped it.

Verified by driving the bootstrap in node against a stubbed localStorage. Behaviour
is byte-identical to before for every input that has prior state — legacy
`solarized`, an explicit `light`+`mono`, and a skin-only `slate` all produce the
same painted result and the same localStorage. The only case that changes is the
empty one, which is the bug:

    before   {} → localStorage {hermes-theme: dark, hermes-skin: default}   server locked out
    after    {} → localStorage {}                                            server wins

Painting is unchanged throughout: a fresh visit still renders dark, since that is
still the fallback.

* test(appearance): pin absent vs explicit appearance state in the bootstrap

Adds the regression coverage asked for in review. Nothing previously
distinguished "no appearance stored" from "user chose these values":
tests/test_1003_appearance_autosave.py covers picker autosave and
tests/test_bugfix_sweep.py covers guarded storage access, but neither fails if
the bootstrap starts fabricating a choice again.

Executes the REAL bootstrap, lifted out of static/index.html, in node against a
stubbed localStorage and documentElement — the same harness used to verify the
fix, now committed instead of run by hand. Covers all three states the review
called out, plus the pre-paint assertion:

  - both keys absent   -> no setItem at all, AND the dark class is still applied
                          (the fix changes persistence only, never the paint)
  - either key present -> both values normalised and persisted
  - legacy names       -> solarized/slate/monokai/nord/oled still migrate, and
                          the DERIVED skin is still written

That last group is why the guard is on the pair rather than per key: with
`hermes-theme=solarized` and no `hermes-skin`, the write of the derived skin is
the only thing that persists the mapping, so a per-key guard would silently drop
it on the next load.

Verified as a regression test, not a tautology: against the pre-fix
static/index.html, test_fresh_browser_writes_nothing and
test_guard_is_on_the_pair_not_per_key both FAIL, while the other nine still pass
— the unfixed bootstrap does still paint dark and does still persist
migrations, so only the buggy behaviour differs.

One harness note worth keeping: the driver and the extracted script are passed
as FILES (`node <driver> <script>`). Passing the script via `node -e ... --
<script>` shifts process.argv, so the driver eval'd the JSON scenario instead of
the bootstrap — and since `eval('{}')` is a valid empty block rather than a
syntax error, the empty-store case passed while executing nothing. _run() now
asserts the bootstrap produced some effect.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(appearance): keep `var themes=` intact so the skin tests still find the block

CI caught this; review could not. `tests/test_sienna_skin.py` (x2) and
`tests/test_catppuccin_skin.py` locate the inline bootstrap with

    init_script_idx = INDEX_HTML.find("var themes=")
    end_idx = INDEX_HTML.find("</script>", init_script_idx)
    init_block = INDEX_HTML[init_script_idx:end_idx]

Declaring `_hadAppearance` as the first `var` in that chain turned `var themes=`
into `,themes=`, so `find()` returned -1, `init_block` came out EMPTY, and three
assertions failed against the empty string — including "Default theme must remain
'dark'", which read as though this PR had changed the default. It had not: the
default is untouched, the block simply could not be located any more.

Splitting the declaration (`;var themes=` instead of `,themes=`) restores the
literal. Both are `var` in the same function scope, so behaviour is identical —
the regression module still passes unchanged, including the empty-store case
(no writes, dark class still applied) and all five legacy migrations.

Failed on shards 3 and 4 across 3.11/3.12/3.13 while the same shards passed on
master, which is what pinned it to this change rather than flake.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: Hermes Agent <hermes@aip.de>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: nesquena-hermes <nesquena+hermes@gmail.com>
2026-08-14 18:51:41 +00:00
..