Skip to content

fix(desktop): preserve onboarding state on transient runtime probe failures - #51884

Merged
kshitijk4poor merged 4 commits into
NousResearch:mainfrom
kshitijk4poor:salvage/pr-51434
Jun 24, 2026
Merged

kshitijk4poor merged 4 commits into
NousResearch:mainfrom
kshitijk4poor:salvage/pr-51434

Conversation

@kshitijk4poor

Copy link
Copy Markdown

Summary

Desktop onboarding no longer downgrades configured: true to false when gateway runtime probes timeout transiently — returning users stay in the chat UI instead of seeing the onboarding overlay flash on every restart.

Root cause: refreshOnboarding() treated ALL non-ready results as authoritative, including source: 'fallback' (both probes timed out / transport failure). A transient gateway timeout during startup would clear the optimistic cache and force the blocking onboarding overlay even though the user had already completed setup.

Changes

  • apps/desktop/src/store/onboarding.ts — new shouldPreserveConfiguredOnFallback() helper checks runtime.source === 'fallback' && state.configured === true && !state.requested. When true, refreshOnboarding() returns early without downgrading, and fires a non-blocking notify() with stable id 'runtime-not-ready' (deduplicates across repeated calls during an outage).
  • apps/desktop/src/store/onboarding.test.ts — 6 new tests covering: fallback preserves configured, notification fires, requested=true overrides preservation, first-run still downgrades, plus the pre-existing tests remain green.

Validation

Before After
Transient timeout + returning user Overlay re-appears Stays in chat, toast notification
Transient timeout + first run Onboarding shows Onboarding shows (unchanged)
Transient timeout + user opens onboarding Onboarding shows Onboarding shows (unchanged)
Repeated timeouts N/A Single toast (deduped by stable id)
Tests 5 passed 11 passed

Credits

Closes #37554

infinitycrew39 and others added 4 commits June 24, 2026 18:09
When shouldPreserveConfiguredOnFallback keeps configured=true, also call
notifyError('runtime-not-ready', ...) so the user knows the backend wasn't
verified instead of silently proceeding. Adapted from @mohamedorigami-jpg's
approach in PR NousResearch#37634.
hermes-pr-review findings:
- notifyError('runtime-not-ready', msg) misused the (error, fallback) API:
  the key became the notification body and the message became the title.
  Switch to notify({ id, kind, title, message }) which puts content in the
  right slots.
- The stable id 'runtime-not-ready' deduplicates: notify() replaces by id,
  so repeated refreshOnboarding calls during an outage no longer stack
  up to 4 persistent error toasts.
- Remove dead !state.manual guard from shouldPreserveConfiguredOnFallback:
  refreshOnboarding already short-circuits on manual before the helper.
- Test: seed localStorage with '1' before asserting it survives (was testing
  the wrong invariant — null in, null out).
- Test: use static import for spy instead of fragile await import.
- Test: add negative case for requested=true + configured=true (should
  still downgrade — requested overrides preservation).
@kshitijk4poor
kshitijk4poor merged commit a4a74ca into NousResearch:main Jun 24, 2026
18 checks passed
@alt-glitch alt-glitch added type/bug Something isn't working comp/desktop Electron desktop app (apps/desktop/*) P3 Low — cosmetic, nice to have labels Jun 24, 2026
@kshitijk4poor
kshitijk4poor deleted the salvage/pr-51434 branch August 5, 2026 07:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/desktop Electron desktop app (apps/desktop/*) 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.

Desktop: onboarding overlay re-appears on every app restart

3 participants