Skip to content

fix(desktop): avoid false 'needs setup' on transient runtime-check timeouts - #51434

Closed
infinitycrew39 wants to merge 2 commits into
NousResearch:mainfrom
infinitycrew39:fix/desktop-runtime-fallback-guard
Closed

fix(desktop): avoid false 'needs setup' on transient runtime-check timeouts#51434
infinitycrew39 wants to merge 2 commits into
NousResearch:mainfrom
infinitycrew39:fix/desktop-runtime-fallback-guard

Conversation

@infinitycrew39

Copy link
Copy Markdown
Contributor

Summary

Desktop onboarding could incorrectly downgrade a previously-configured install to "needs setup" when runtime readiness probes returned only fallback errors (for example transient request timed out: setup.runtime_check / reconnect windows).

This showed up as random status flapping (connected -> needs setup -> offline) and could block composer usage even though credentials were already valid.

Root Cause

refreshOnboarding() treated non-ready fallback results as authoritative and immediately set configured=false.

Fallback results are intentionally non-authoritative (transport timeout/disconnect), so using them to downgrade state causes false negatives.

Fix

  • Preserve configured=true when readiness result is fallback-only and onboarding was not explicitly requested/manual.
  • Keep existing strict behavior for first-run/manual/requested onboarding flows.

Tests

Added regression coverage:

  1. Previously configured desktop install is not downgraded on fallback timeout.
  2. Unconfigured/requested onboarding still surfaces correctly on fallback failure.

Commits

  1. fix(desktop): keep configured onboarding state on fallback runtime probes
  2. test(desktop): cover fallback timeout onboarding downgrade regression

@alt-glitch alt-glitch added type/bug Something isn't working comp/desktop Electron desktop app (apps/desktop/*) P3 Low — cosmetic, nice to have duplicate This issue or pull request already exists labels Jun 23, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Duplicate of #37634 — same fix for the same mechanism: refreshOnboarding() in apps/desktop/src/store/onboarding.ts treats non-authoritative fallback runtime-check results (transient timeouts) as authoritative and downgrades a configured install to "needs setup". #37634 (earlier open PR, fixes #37554) keeps the onboarding state sticky across the same transient failures.

@kshitijk4poor

Copy link
Copy Markdown
Collaborator

Merged via PR #51884 — your shouldPreserveConfiguredOnFallback helper and initial tests were cherry-picked with authorship preserved. The salvage added a notify() call with a stable id for non-blocking dedup'd notifications, removed a dead !state.manual guard, and added two more negative-case tests.

Thanks for the clean, well-targeted fix! Your approach of gating on runtime.source === 'fallback' was the right call — it correctly distinguishes transient timeouts from authoritative "not configured" responses.

Also credited in the PR body: @mohamedorigami-jpg (PR #37634) who independently proposed the notification-on-preserve idea.

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/*) duplicate This issue or pull request already exists 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.

3 participants