Skip to content

fix(desktop): keep onboarding cache sticky across transient runtime failures (fixes #37554) - #37634

Closed
Morad37 wants to merge 1 commit into
NousResearch:mainfrom
Morad37:fix/37554-onboarding-regression-cache
Closed

fix(desktop): keep onboarding cache sticky across transient runtime failures (fixes #37554)#37634
Morad37 wants to merge 1 commit into
NousResearch:mainfrom
Morad37:fix/37554-onboarding-regression-cache

Conversation

@Morad37

@Morad37 Morad37 commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Stops refreshOnboarding() from downgrading the optimistic onboarding cache from true to false on transient runtime probe failures. A returning user whose runtime check transiently fails (gateway still warming up, momentary timeout, runtime/credentials mismatch) used to see the onboarding overlay flash on every reload and have to re-add their provider, even though the durable bootstrap marker on disk said onboarding was complete.

Why

localStorage['hermes-desktop-onboarded-v1'] is only an optimistic cache. Every gateway open triggers refreshOnboarding(), which calls evaluateRuntimeReadiness() and on failure ran:

writeCachedConfigured(false)
patch({ configured: false, reason })

That overwrite defeated the cache: the next reload would re-evaluate and re-flip. The bootstrap marker (.hermes-bootstrap-complete) is the durable source of truth, but the renderer can't see it directly, so the right fix is to keep the cache sticky once it has been set and surface the runtime failure as a notification rather than a full re-onboarding.

Changes

  • apps/desktop/src/store/onboarding.ts — in refreshOnboarding(), when the runtime probe returns ready: false, only downgrade configured to false if neither the in-memory state nor the localStorage cache already says configured === true. If we were already configured, keep the cache and call notifyError(...) so the user still sees a heads-up that the gateway isn't reachable.
  • apps/desktop/src/store/onboarding.test.ts — adds two regression tests:
    • Returning user with cache configured === true + transient runtime failure: store stays configured: true, cache is preserved, no API refresh fires.
    • Brand new install (no prior cache) with same transient failure: still flips to configured: false and refreshes providers.

How to test

  1. cd apps/desktop && npx vitest run --environment jsdom src/store/onboarding.test.ts — all 5 tests pass.
  2. npx eslint src/store/onboarding.ts src/store/onboarding.test.ts — 0 errors (5 pre-existing padding-line warnings on lines unrelated to this change).
  3. npx tsc --noEmit -p tsconfig.json — 0 errors in changed files. (One pre-existing TS2307 in src/app/messaging/platform-icon.tsx is unrelated.)

Notes

  • Behaviour change is intentionally narrow: only affects the path where runtime.ready === false AND the user has previously completed onboarding. First-run onboarding still works exactly as before.
  • The fix is layered correctly — the bigger "use the durable bootstrap marker" refactor called out in the issue would be a separate, much larger change. This is the smallest, safest fix that resolves the reported regression.

Closes #37554

…ailures (fixes NousResearch#37554)

refreshOnboarding() used to unconditionally write cached configured=false and
flip the store state to false on any runtime probe failure, overwriting the
optimistic localStorage cache from a previous successful onboarding. Returning
users whose runtime check transiently failed (gateway still warming up,
momentary timeout, runtime/credentials mismatch) would see the onboarding
overlay flash on every reload and have to re-add their provider.

Keep the cache sticky once configured=true, surface the runtime failure as a
notification so the user still knows something is off, and only flip
configured=false on a true first run with no prior cache.
@kshitijk4poor

Copy link
Copy Markdown
Collaborator

Merged via PR #51884 — your idea of surfacing a notification on the preserve path (rather than silently returning) was incorporated into the salvage. Your PR #37634 correctly identified the bug and proposed the right product behavior (non-blocking notification + keep configured). The salvage used @infinitycrew39's more surgical runtime.source === 'fallback' check (PR #51434) as the core mechanism, since it avoids masking authoritative "not configured" responses, but your notification approach was adopted directly.

Both approaches are credited in the PR #51884 body. Thanks for the report and the fix!

kshitijk4poor added a commit that referenced this pull request Jun 24, 2026
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 #37634.
pai-scaffolde pushed a commit to pai-scaffolde/hermes-agent that referenced this pull request Jun 28, 2026
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.
waefrebeorn pushed a commit to waefrebeorn/slermes that referenced this pull request Jul 2, 2026
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.
habarmc1223-sudo pushed a commit to habarmc1223-sudo/hermes-agent-fluxmem that referenced this pull request Jul 8, 2026
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.
santhreal pushed a commit to santhreal/hermes-agent that referenced this pull request Jul 13, 2026
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.
Gravezzz pushed a commit to Gravezzz/hermes-agent that referenced this pull request Jul 21, 2026
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.
leewenjie pushed a commit to leewenjie/hermes-agent that referenced this pull request Aug 7, 2026
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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