Skip to content

fix(desktop): keep Dark mode across Bot Mode new chat - #101528

Merged
OutThisLife merged 2 commits into
NousResearch:mainfrom
FalconOrtiz:fix/fe-101216-bot-new-chat-theme
Oct 6, 2026
Merged

OutThisLife merged 2 commits into
NousResearch:mainfrom
FalconOrtiz:fix/fe-101216-bot-new-chat-theme

Conversation

@FalconOrtiz

Copy link
Copy Markdown

Why

Bot Mode New chat with this bot activates the bot gateway profile. That profile usually has no appearance of its own. When the launch profile is not default, Settings → Appearance Dark only wrote the per-profile record, so the hop resolved system and a light OS flipped the window to Light (#101216).

Scope

Out of scope: $appearanceProfile decoupling (issue suggestion 1), Theme Scope Shared (#96271), Appearance caption copy.

Tradeoffs

Mirroring into the global slot means the last named-profile Appearance pick becomes the fallback for never-themed profiles. Explicit per-profile overrides still win. This is smaller than teaching ThemeProvider to ignore Bot Mode gateway activations.

Blast Radius

Desktop theme/mode persistence only. Users with one unanimous mode across profiles get that mode on never-themed bots and on cold launch after a bot hop. Installs with conflicting per-profile modes keep those overrides and do not auto-promote.

Verification

cd apps/desktop
npx vitest run src/themes/profile-theme.test.ts src/themes/context.test.tsx

20 passed, exit 0.

@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/desktop Electron desktop app (apps/desktop/*) area/profiles Multi-profile isolation, HERMES_HOME scoping labels Sep 2, 2026
@Enough1122

Copy link
Copy Markdown

AI code review — automated review for reference; please use your judgment.

Overall: Named-profile appearance assigns now mirror into the global slot (and a unanimous pre-existing record promotes into it), so a Bot Mode hop onto a never-themed bot inherits the look the user picked instead of resolving system (#101216).

Correctness:

  • apps/desktop/src/themes/context.tsx:profilePref — default still writes only the legacy slot; named assigns write their record entry plus the legacy slot; explicit per-profile overrides still win on resolve (test-pinned).
  • promoteUnanimousLegacy — no-ops when the legacy slot is set or records disagree; unanimous promotion covers pre-per-profile installs.

Non-blocking nit:

  • resolve() now has a write side effect (promotion persists on read). Harmless here (idempotent, same value), but a reader-triggered write can surprise — e.g. under concurrent windows racing the first resolve; last-writer-wins with identical values, so no corruption, just noting it.

Verdict: LGTM.

@FalconOrtiz

Copy link
Copy Markdown
Author

Requesting squash-merge per engineer agreement (2026-09-04): AI-review merge-ready pack; nits non-blocking; no rewrites.

This PR is in the agreed pack with #101523 #101528 #101529 #101530 #101531 #101537 #101544 #101547 #101548 #101549 #101551.

Out of pack (do not block this merge): #101564, #101796/#101091, #101318.

@kvnloo

kvnloo commented Sep 9, 2026

Copy link
Copy Markdown

Exact-head review of 7335bd7.

Keep: named-profile assign mirrors into the global/legacy slot, plus inherit/override tests (#101216):

  • never-themed bot inherits Dark set on the launch profile
  • explicit per-profile override still wins
  • unanimous pre-existing per-profile mode promotes into an empty global slot
    Without the mirror, Bot Mode hopping onto a never-themed bot falls through to system.

Check:

  • last-assign fallback: every named assign writes persistString(legacy, value), so the global slot is whichever profile was assigned last. After forge=dark then atlas=light, a third never-themed profile inherits light. Confirm that is the intended hop, not "first launch profile."
  • promoteUnanimousLegacy runs inside resolve (write-on-read). A getter that persists is surprising if two profiles already disagree it no-ops, but a single stored profile will stamp legacy on first resolve of any name. Worth a one-line note if that migration should be a one-shot boot path instead of every resolve.

No competing PR from me.

@FalconOrtiz

Copy link
Copy Markdown
Author

@kvnloo Confirmed. Last named-profile assign is the intended hop, not the first launch profile.

assign writes persistString(legacy, value) on every named pick, so after forge=dark then atlas=light a third never-themed profile inherits light. Pack agreement 2026-09-04 said no rewrites, so that hop stays. A focused test now pins it (lets a never-themed profile inherit the last named-profile assign).

promoteUnanimousLegacy stays a write-on-read from resolve. It is idempotent. It no-ops when the legacy slot is set or when records disagree. A one-line why is on the helper. Leaving it on every resolve keeps Bot Mode hops and cold start on the same path.

@FalconOrtiz
FalconOrtiz force-pushed the fix/fe-101216-bot-new-chat-theme branch from a7e731b to ada0347 Compare September 11, 2026 13:12
@FalconOrtiz

Copy link
Copy Markdown
Author

Awaiting maintainer. No merge from this side.

HEAD is still ada0347. GitHub reports MERGEABLE with mergeStateStatus BLOCKED. There are no review threads. watch-pr --status-only ended as BLOCKER status-query because the check rollup is empty.

I inspected the Actions tab. CI, Nix flake check, and Docker on this SHA are awaiting maintainer approval. Jobs have not started (0 jobs per run). That is not a Sonar failure and not a test failure. Approving those runs needs admin on NousResearch/hermes-agent. FalconOrtiz got HTTP 403.

This branch was rebased onto origin/main 8706517. Main's stored() / storedSkin() / committedName stayed. Named assign still mirrors persistString(legacy, value). promoteUnanimousLegacy is still write-on-read and now runs inside stored(). Local e2e smoke for this SHA is e2e_smoke_ok (C:\Users\falco\.cursor\orchestrate\hermes-agent\reports\e2e-smoke-101528-ada03473.md). There is no Sonar workflow in this repo, so excellency is N/A until checks exist.

Enough1122's LGTM nit still stands. Promotion persists on read. Pack agreement 2026-09-04 said nits are non-blocking and no rewrites. kvnloo's last-assign question was answered earlier. Last named assign is the intended hop. The four #101216 tests remain, including that hop.

origin/main is still 8706517 and is an ancestor of HEAD. GitHub reports no merge conflicts. I did not rebase again.

Approve the waiting workflows, then review and land if you want this in.

@FalconOrtiz

Copy link
Copy Markdown
Author

Follow-up after rebase onto current main (ada0347390).

1d1411df9c split the old resolve helper into stored() / storedSkin(). Promotion now runs inside stored(), because ThemeProvider paints skin through storedSkin then skinPref.stored, not resolve. resolve is normalize(stored()), so Bot Mode hops still see the promoted global slot.

Last-assign hop is unchanged. Named assign still writes the global slot. The last-assign test still pins forge=dark then atlas=light, so a never-themed profile inherits light.

The earlier note that promotion lived only on resolve is stale after this rebase.

@alt-glitch alt-glitch added P2 Medium — degraded but workaround exists sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades P3 Low — cosmetic, nice to have and removed P3 Low — cosmetic, nice to have P2 Medium — degraded but workaround exists labels Sep 11, 2026
FalconOrtiz and others added 2 commits October 6, 2026 10:58
Named-profile appearance picks now mirror into the global fallback, and a unanimous pre-existing per-profile mode promotes into an empty global slot. Bot Mode gateway hops onto never-themed bots inherit the user's mode instead of resolving system (NousResearch#101216).

Co-authored-by: Cursor <cursoragent@cursor.com>
Named-profile assign writes the global slot on every pick. After forge=dark then atlas=light, a never-themed profile inherits light. A focused test pins that hop. promoteUnanimousLegacy persists from stored. The write is idempotent and a no-op when records disagree.
@OutThisLife
OutThisLife force-pushed the fix/fe-101216-bot-new-chat-theme branch from ada0347 to 54913d3 Compare October 6, 2026 16:04
@OutThisLife
OutThisLife merged commit 14e941b into NousResearch:main Oct 6, 2026
37 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/profiles Multi-profile isolation, HERMES_HOME scoping comp/desktop Electron desktop app (apps/desktop/*) P3 Low — cosmetic, nice to have 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.

5 participants