Skip to content

fix(desktop): notify when a profile switch fails instead of silently reverting - #89634

Closed
chelsealong wants to merge 1 commit into
NousResearch:mainfrom
chelsealong:fix/profile-switch-fail-notify-89622
Closed

fix(desktop): notify when a profile switch fails instead of silently reverting#89634
chelsealong wants to merge 1 commit into
NousResearch:mainfrom
chelsealong:fix/profile-switch-fail-notify-89622

Conversation

@chelsealong

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes the desktop profile switcher silently failing: clicking a profile shows the "Waking up …" overlay, then it just disappears and nothing switches, with no error and no way to tell what happened.

Related Issue

Fixes #89622

Root Cause

selectProfile() / newSessionInProfile() (apps/desktop/src/store/profile.ts) call ensureGatewayProfile(target) fire-and-forget (void ensureGatewayProfile(target)), and ensureGatewayProfile itself swallows a failed descriptor lookup:

})().catch(() => {
  // Descriptor lookup failed: the switch fails as a unit. Nothing was
  // published, so every atom still consistently describes the previous
  // profile; the user can retry the switch.
})

That "publish nothing on failure" behavior is correct (it's what #46651's regression test in profile.test.ts already locks in) — but it is also completely silent. When resolveConnectionForProfile/prepareGatewayForProfile reject (backend unreachable, profile deleted mid-dial, etc.), the ChatSwapOverlay "waking up" indicator (driven by $gatewaySwapTarget, cleared in the finally regardless of outcome) just fades out, $activeGatewayProfile never moves, and the caller never learns the switch was declined. From the user's side that reads exactly like #89622: "it says waking up... but it didn't actually switch."

Changes Made

  • apps/desktop/src/store/profile.tsensureGatewayProfile's failure path now calls notifyError() (the app's existing top-center toast) with a new translated message, so a failed switch is visibly reported instead of silently reverting.
  • apps/desktop/src/i18n/types.ts, apps/desktop/src/i18n/en.ts, apps/desktop/src/i18n/zh.ts — add the composer.profileSwitchFailed string (the two locales that declare the full Translations type directly; the other locales fall back to English via defineLocale).
  • apps/desktop/src/store/profile.test.ts — new regression test asserting notifyError fires with the failing error when the descriptor fetch rejects.

How to Test

Commands run from apps/desktop:

npx vitest run --project ui src/store/profile.test.ts
npm run typecheck
npm run lint

Results:

  • src/store/profile.test.ts: 16 passed (15 pre-existing + 1 new).
  • A/B guard: with only the profile.ts hunk reverted (git checkout HEAD~1 -- apps/desktop/src/store/profile.ts apps/desktop/src/i18n/en.ts apps/desktop/src/i18n/types.ts), the new test notifies the user when a fire-and-forget switch fails (#89622) FAILS (expected "vi.fn()" to be called ... Number of calls: 0); all other tests in the file still pass.
  • npm run typecheck: passed (tsc -p . --noEmit && tsc -p tsconfig.electron.json --noEmit && tsc -p tsconfig.e2e.json --noEmit).
  • npm run lint: 0 errors, 129 pre-existing warnings (none in touched files).

Checklist

Code

Documentation & Housekeeping

  • No config keys changed — N/A
  • No architecture/workflow change — N/A
  • Cross-platform: no OS-specific code — N/A

Disclosure

This change was prepared with AI assistance (Claude Code), including root-cause analysis of the swallowed rejection path, the fix, and the regression test. All commands above were run and their real output is reported here.

…reverting

ensureGatewayProfile's fire-and-forget catch (used by selectProfile and
newSessionInProfile) swallowed a failed descriptor lookup entirely: no
atom is published, which is correct, but nothing told the user either.
The rail's "waking up" overlay just vanished and the active profile
stayed put with no explanation, which reads as a broken click (NousResearch#89622).

Surface the failure via the existing notification system instead.
@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 sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Aug 19, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related: #89621 and #89609 also address silent declined desktop profile switches through different error-publication paths. #89621 is the earlier broader candidate because it also handles a wedged switch; please review the approaches together.

@teknium1

Copy link
Copy Markdown
Contributor

Closing — the seams this PR hardens were rewritten during the resolution of #89622, and the specific failure it surfaces no longer exists.

Sequence on main: the atomic-publish series was reverted (#89785), profile switching was re-landed with fail-open semantics and an activation lease preventing mid-dial socket disposal (#89797), and the underlying release-build breakage turned out to be nanostores 1.4.0's @__NO_SIDE_EFFECTS__ annotation on batch() letting Rollup strip the switch publication from minified bundles — fixed by the 1.4.2 bump in #89875.

On the current code a switch cannot silently decline (the decline path is gone), and descriptor-lookup failures are logged rather than swallowed. If you see a switch-failure UX gap remaining on latest main — e.g. a user-visible toast rather than a console warning — a fresh PR against the current seams would be welcome. Thanks for the work here; the silent-failure framing was correct and informed the final design.

@teknium1 teknium1 closed this Aug 19, 2026
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-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Profile switching is broken!

3 participants